diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index d94be16f5b..0575e02301 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -27,6 +27,7 @@ from xmodule.tabs import CourseTab, CourseTabList, InvalidTabsException from openedx.core.lib.course_tabs import CourseTabPluginManager from openedx.core.djangoapps.credit.api import is_credit_course, get_credit_requirements from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements +from openedx.core.djangoapps.content.course_structures.api.v0 import api, errors from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError from opaque_keys import InvalidKeyError @@ -48,6 +49,7 @@ from contentstore.utils import ( get_lms_link_for_item, reverse_course_url, reverse_library_url, + reverse_usage_url, reverse_url, remove_all_instructors, ) @@ -473,6 +475,44 @@ def _get_rerun_link_for_item(course_key): return reverse_course_url('course_rerun_handler', course_key) +def _deprecated_blocks_info(course_module, deprecated_block_types): + """ + Returns deprecation information about `deprecated_block_types` + + Arguments: + course_module (CourseDescriptor): course object + deprecated_block_types (list): list of deprecated blocks types + + Returns: + Dict with following keys: + block_types (list): list containing types of all deprecated blocks + block_types_enabled (bool): True if any or all `deprecated_blocks` present in Advanced Module List else False + blocks (list): List of `deprecated_block_types` component names and their parent's url + advance_settings_url (str): URL to advance settings page + """ + data = { + 'block_types': deprecated_block_types, + 'block_types_enabled': any( + block_type in course_module.advanced_modules for block_type in deprecated_block_types + ), + 'blocks': [], + 'advance_settings_url': reverse_course_url('advanced_settings_handler', course_module.id) + } + + try: + structure_data = api.course_structure(course_module.id, block_types=deprecated_block_types) + except errors.CourseStructureNotAvailableError: + return data + + blocks = [] + for block in structure_data['blocks'].values(): + blocks.append([reverse_usage_url('container_handler', block['parent']), block['display_name']]) + + data['blocks'].extend(blocks) + + return data + + @login_required @ensure_csrf_cookie def course_index(request, course_key): @@ -500,6 +540,8 @@ def course_index(request, course_key): except (ItemNotFoundError, CourseActionStateItemNotFoundError): current_action = None + deprecated_blocks_info = _deprecated_blocks_info(course_module, settings.DEPRECATED_BLOCK_TYPES) + return render_to_response('course_outline.html', { 'context_course': course_module, 'lms_link': lms_link, @@ -513,6 +555,7 @@ def course_index(request, course_key): 'course_release_date': course_release_date, 'settings_url': settings_url, 'reindex_link': reindex_link, + 'deprecated_blocks_info': deprecated_blocks_info, 'notification_dismiss_url': reverse_course_url( 'course_notifications_handler', current_action.course_key, diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 2c7ee14748..fa3a97cffb 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -1,10 +1,10 @@ """ Unit tests for getting the list of courses and the course outline. """ +import ddt import json import lxml import datetime -import os import mock import pytz @@ -14,8 +14,10 @@ from django.utils.translation import ugettext as _ from contentstore.courseware_index import CoursewareSearchIndexer, SearchIndexingError from contentstore.tests.utils import CourseTestCase -from contentstore.utils import reverse_course_url, reverse_library_url, add_instructor -from contentstore.views.course import course_outline_initial_state, reindex_course_and_check_access +from contentstore.utils import reverse_course_url, reverse_library_url, add_instructor, reverse_usage_url +from contentstore.views.course import ( + course_outline_initial_state, reindex_course_and_check_access, _deprecated_blocks_info +) from contentstore.views.item import create_xblock_info, VisibilityState from course_action_state.managers import CourseRerunUIStateManager from course_action_state.models import CourseRerunState @@ -225,6 +227,7 @@ class TestCourseIndex(CourseTestCase): self.assert_correct_json_response(child_response) +@ddt.ddt class TestCourseOutline(CourseTestCase): """ Unit tests for the course outline. @@ -340,6 +343,146 @@ class TestCourseOutline(CourseTestCase): self.assertEqual(_get_release_date(response), get_default_time_display(self.course.start)) _assert_settings_link_present(response) + def _create_test_data(self, course_module, create_blocks=False, publish=True, block_types=None): + """ + Create data for test. + """ + if create_blocks: + for block_type in block_types: + ItemFactory.create( + parent_location=self.vertical.location, + category=block_type, + display_name='{} Problem'.format(block_type) + ) + + if not publish: + self.store.unpublish(self.vertical.location, self.user.id) + + course_module.advanced_modules.extend(block_types) + + def _verify_deprecated_info(self, course_id, advanced_modules, info, deprecated_block_types): + """ + Verify deprecated info. + """ + expected_blocks = [] + for block_type in deprecated_block_types: + expected_blocks.append( + [ + reverse_usage_url('container_handler', self.vertical.location), + '{} Problem'.format(block_type) + ] + ) + + self.assertEqual(info['block_types'], deprecated_block_types) + self.assertEqual( + info['block_types_enabled'], + any(component in advanced_modules for component in deprecated_block_types) + ) + self.assertItemsEqual(info['blocks'], expected_blocks) + self.assertEqual( + info['advance_settings_url'], + reverse_course_url('advanced_settings_handler', course_id) + ) + + @ddt.data( + {'publish': True}, + {'publish': False}, + ) + @ddt.unpack + def test_verify_deprecated_warning_message_with_single_feature(self, publish): + """ + Verify deprecated warning info for single deprecated feature. + """ + block_types = settings.DEPRECATED_BLOCK_TYPES + course_module = modulestore().get_item(self.course.location) + self._create_test_data(course_module, create_blocks=True, block_types=block_types, publish=publish) + info = _deprecated_blocks_info(course_module, block_types) + self._verify_deprecated_info( + course_module.id, + course_module.advanced_modules, + info, + block_types + ) + + def test_verify_deprecated_warning_message_with_multiple_features(self): + """ + Verify deprecated warning info for multiple deprecated features. + """ + block_types = ['peergrading', 'combinedopenended', 'openassessment'] + course_module = modulestore().get_item(self.course.location) + self._create_test_data(course_module, create_blocks=True, block_types=block_types) + + info = _deprecated_blocks_info(course_module, block_types) + self._verify_deprecated_info(course_module.id, course_module.advanced_modules, info, block_types) + + @ddt.data( + {'delete_vertical': True}, + {'delete_vertical': False}, + ) + @ddt.unpack + def test_deprecated_blocks_list_updated_correctly(self, delete_vertical): + """ + Verify that deprecated blocks list shown on banner is updated correctly. + + Here is the scenario: + This list of deprecated blocks shown on banner contains published + and un-published blocks. That list should be updated when we delete + un-published block(s). This behavior should be same if we delete + unpublished vertical or problem. + """ + block_types = ['peergrading'] + course_module = modulestore().get_item(self.course.location) + + vertical1 = ItemFactory.create( + parent_location=self.sequential.location, category='vertical', display_name='Vert1 Subsection1' + ) + problem1 = ItemFactory.create( + parent_location=vertical1.location, + category='peergrading', + display_name='peergrading problem in vert1', + publish_item=False + ) + + info = _deprecated_blocks_info(course_module, block_types) + # info['blocks'] should be empty here because there is nothing + # published or un-published present + self.assertEqual(info['blocks'], []) + + vertical2 = ItemFactory.create( + parent_location=self.sequential.location, category='vertical', display_name='Vert2 Subsection1' + ) + ItemFactory.create( + parent_location=vertical2.location, + category='peergrading', + display_name='peergrading problem in vert2', + pubish_item=True + ) + # At this point CourseStructure will contain both the above + # published and un-published verticals + + info = _deprecated_blocks_info(course_module, block_types) + self.assertItemsEqual( + info['blocks'], + [ + [reverse_usage_url('container_handler', vertical1.location), 'peergrading problem in vert1'], + [reverse_usage_url('container_handler', vertical2.location), 'peergrading problem in vert2'] + ] + ) + + # Delete the un-published vertical or problem so that CourseStructure updates its data + if delete_vertical: + self.store.delete_item(vertical1.location, self.user.id) + else: + self.store.delete_item(problem1.location, self.user.id) + + info = _deprecated_blocks_info(course_module, block_types) + # info['blocks'] should only contain the info about vertical2 which is published. + # There shouldn't be any info present about un-published vertical1 + self.assertEqual( + info['blocks'], + [[reverse_usage_url('container_handler', vertical2.location), 'peergrading problem in vert2']] + ) + class TestCourseReIndex(CourseTestCase): """ diff --git a/cms/envs/common.py b/cms/envs/common.py index f7485589cf..0bff2b71f5 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1007,3 +1007,8 @@ CREDIT_TASK_MAX_RETRIES = 5 # when a credit provider notifies us that a student has been approved # or denied for credit. CREDIT_PROVIDER_TIMESTAMP_EXPIRATION = 15 * 60 + + +################################ Deprecated Blocks Info ################################ + +DEPRECATED_BLOCK_TYPES = ['peergrading', 'combinedopenended'] diff --git a/cms/static/sass/elements/_system-feedback.scss b/cms/static/sass/elements/_system-feedback.scss index 27240de499..bf5ca44f00 100644 --- a/cms/static/sass/elements/_system-feedback.scss +++ b/cms/static/sass/elements/_system-feedback.scss @@ -937,3 +937,7 @@ body.error { padding: 14px 40px 18px; } } + +.advance-modules-remove-text { + margin-top: ($baseline/2); +} diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index f3246a9e9f..595e083cff 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -6,6 +6,7 @@ import logging from util.date_utils import get_default_time_display from django.utils.translation import ugettext as _ from contentstore.utils import reverse_usage_url +from microsite_configuration import microsite %> <%block name="title">${_("Course Outline")} <%block name="bodyclass">is-signedin course view-outline @@ -50,6 +51,53 @@ from contentstore.utils import reverse_usage_url %endif + + %if deprecated_blocks_info.get('blocks') or deprecated_blocks_info.get('block_types_enabled'): + <% + platform_name = microsite.get_value('platform_name', settings.PLATFORM_NAME) + %> +
+
+ ${_("Warning")} + +
+

${_("This course uses features that are no longer supported.")}

+ + %if deprecated_blocks_info.get('blocks'): +
+

${_("You must delete or replace the following components.")}

+ +
+ %endif + + % if deprecated_blocks_info.get('block_types_enabled'): +
+

+ ${_("To avoid errors, {platform_name} strongly recommends that you remove unsupported features from the course advanced settings. To do this, go to the {link_start}Advanced Settings page{link_end}, locate the \"Advanced Module List\" setting, and then delete the following modules from the list.").format( + platform_name=platform_name, + link_start=''.format(advance_settings_url=deprecated_blocks_info['advance_settings_url']), link_end="" + )} +

+ +
+ % endif +
+
+
+ %endif + <%block name="content"> diff --git a/common/test/acceptance/pages/studio/overview.py b/common/test/acceptance/pages/studio/overview.py index 387e42e614..842bd0d0dc 100644 --- a/common/test/acceptance/pages/studio/overview.py +++ b/common/test/acceptance/pages/studio/overview.py @@ -586,6 +586,62 @@ class CourseOutlinePage(CoursePage, CourseOutlineContainer): """ return self.q(css=".license-value").first.text[0] + @property + def deprecated_warning_visible(self): + """ + Returns true if the deprecated warning is visible. + """ + return self.q(css='.wrapper-alert-error.is-shown').is_present() + + @property + def warning_heading_text(self): + """ + Returns deprecated warning heading text. + """ + return self.q(css='.warning-heading-text').text[0] + + @property + def components_list_heading(self): + """ + Returns deprecated warning component list heading text. + """ + return self.q(css='.components-list-heading-text').text[0] + + @property + def modules_remove_text_shown(self): + """ + Returns True if deprecated warning advance modules remove text is visible. + """ + return self.q(css='.advance-modules-remove-text').visible + + @property + def modules_remove_text(self): + """ + Returns deprecated warning advance modules remove text. + """ + return self.q(css='.advance-modules-remove-text').text[0] + + @property + def components_visible(self): + """ + Returns True if components list visible. + """ + return self.q(css='.components-list').visible + + @property + def components_display_names(self): + """ + Returns deprecated warning components display name list. + """ + return self.q(css='.components-list li>a').text + + @property + def deprecated_advance_modules(self): + """ + Returns deprecated advance modules list. + """ + return self.q(css='.advance-modules-list li').text + class CourseOutlineModal(object): MODAL_SELECTOR = ".wrapper-modal-window" diff --git a/common/test/acceptance/tests/studio/test_studio_outline.py b/common/test/acceptance/tests/studio/test_studio_outline.py index 926038d5b1..3d156d9515 100644 --- a/common/test/acceptance/tests/studio/test_studio_outline.py +++ b/common/test/acceptance/tests/studio/test_studio_outline.py @@ -1,12 +1,14 @@ """ Acceptance tests for studio related to the outline page. """ +import json from datetime import datetime, timedelta import itertools from pytz import UTC from bok_choy.promise import EmptyPromise from nose.plugins.attrib import attr +from ...pages.studio.settings_advanced import AdvancedSettingsPage from ...pages.studio.overview import CourseOutlinePage, ContainerPage, ExpandCollapseLinkState from ...pages.studio.utils import add_discussion, drag, verify_ordering from ...pages.lms.courseware import CoursewarePage @@ -37,6 +39,9 @@ class CourseOutlineTest(StudioCourseTest): self.course_outline_page = CourseOutlinePage( self.browser, self.course_info['org'], self.course_info['number'], self.course_info['run'] ) + self.advanced_settings = AdvancedSettingsPage( + self.browser, self.course_info['org'], self.course_info['number'], self.course_info['run'] + ) def populate_course_fixture(self, course_fixture): """ Install a course with sections/problems, tabs, updates, and handouts """ @@ -1578,3 +1583,142 @@ class PublishSectionTest(CourseOutlineTest): unit = subsection.expand_subsection().unit(UNIT_NAME) return (section, subsection, unit) + + +@attr('shard_3') +class DeprecationWarningMessageTest(CourseOutlineTest): + """ + Feature: Verify deprecation warning message. + """ + HEADING_TEXT = 'This course uses features that are no longer supported.' + COMPONENT_LIST_HEADING = 'You must delete or replace the following components.' + ADVANCE_MODULES_REMOVE_TEXT = ('To avoid errors, edX strongly recommends that you remove unsupported features ' + 'from the course advanced settings. To do this, go to the Advanced Settings ' + 'page, locate the "Advanced Module List" setting, and then delete the following ' + 'modules from the list.') + + def _add_deprecated_advance_modules(self, block_types): + """ + Add `block_types` into `Advanced Module List` + + Arguments: + block_types (list): list of block types + """ + self.advanced_settings.visit() + self.advanced_settings.set_values({"Advanced Module List": json.dumps(block_types)}) + + def _create_deprecated_components(self): + """ + Create deprecated components. + """ + parent_vertical = self.course_fixture.get_nested_xblocks(category="vertical")[0] + + self.course_fixture.create_xblock( + parent_vertical.locator, + XBlockFixtureDesc('combinedopenended', "Open", data=load_data_str('ora_peer_problem.xml')) + ) + self.course_fixture.create_xblock(parent_vertical.locator, XBlockFixtureDesc('peergrading', 'Peer')) + + def _verify_deprecation_warning_info( + self, + deprecated_blocks_present, + components_present, + components_display_name_list=None, + deprecated_modules_list=None + ): + """ + Verify deprecation warning + + Arguments: + deprecated_blocks_present (bool): deprecated blocks remove text and + is list is visible if True else False + components_present (bool): components list shown if True else False + components_display_name_list (list): list of components display name + deprecated_modules_list (list): list of deprecated advance modules + """ + self.assertTrue(self.course_outline_page.deprecated_warning_visible) + self.assertEqual(self.course_outline_page.warning_heading_text, self.HEADING_TEXT) + self.assertEqual(self.course_outline_page.modules_remove_text_shown, deprecated_blocks_present) + if deprecated_blocks_present: + self.assertEqual(self.course_outline_page.modules_remove_text, self.ADVANCE_MODULES_REMOVE_TEXT) + self.assertEqual(self.course_outline_page.deprecated_advance_modules, deprecated_modules_list) + + self.assertEqual(self.course_outline_page.components_visible, components_present) + if components_present: + self.assertEqual(self.course_outline_page.components_list_heading, self.COMPONENT_LIST_HEADING) + self.assertItemsEqual(self.course_outline_page.components_display_names, components_display_name_list) + + def test_no_deprecation_warning_message_present(self): + """ + Scenario: Verify that deprecation warning message is not shown if ORA1 + advance modules are not present and also no ORA1 component exist in + course outline. + + When I goto course outline + Then I don't see ORA1 deprecated warning + """ + self.course_outline_page.visit() + self.assertFalse(self.course_outline_page.deprecated_warning_visible) + + def test_deprecation_warning_message_present(self): + """ + Scenario: Verify deprecation warning message if ORA1 advance modules + and ORA1 components are present. + + Given I have ORA1 advance modules present in `Advanced Module List` + And I have created 2 ORA1 components + When I go to course outline + Then I see ORA1 deprecated warning + And I see correct ORA1 deprecated warning heading text + And I see correct ORA1 deprecated warning advance modules remove text + And I see list of ORA1 components with correct display names + """ + self._add_deprecated_advance_modules(block_types=['peergrading', 'combinedopenended']) + self._create_deprecated_components() + self.course_outline_page.visit() + self._verify_deprecation_warning_info( + deprecated_blocks_present=True, + components_present=True, + components_display_name_list=['Open', 'Peer'], + deprecated_modules_list=['peergrading', 'combinedopenended'] + ) + + def test_warning_with_ora1_advance_modules_only(self): + """ + Scenario: Verify that deprecation warning message is shown if only + ORA1 advance modules are present and no ORA1 component exist. + + Given I have ORA1 advance modules present in `Advanced Module List` + When I go to course outline + Then I see ORA1 deprecated warning + And I see correct ORA1 deprecated warning heading text + And I see correct ORA1 deprecated warning advance modules remove text + And I don't see list of ORA1 components + """ + self._add_deprecated_advance_modules(block_types=['peergrading', 'combinedopenended']) + self.course_outline_page.visit() + self._verify_deprecation_warning_info( + deprecated_blocks_present=True, + components_present=False, + deprecated_modules_list=['peergrading', 'combinedopenended'] + ) + + def test_warning_with_ora1_components_only(self): + """ + Scenario: Verify that deprecation warning message is shown if only + ORA1 component exist and no ORA1 advance modules are present. + + Given I have created two ORA1 components + When I go to course outline + Then I see ORA1 deprecated warning + And I see correct ORA1 deprecated warning heading text + And I don't see ORA1 deprecated warning advance modules remove text + And I see list of ORA1 components with correct display names + """ + self._create_deprecated_components() + self.course_outline_page.visit() + self._verify_deprecation_warning_info( + deprecated_blocks_present=False, + components_present=True, + components_display_name_list=['Open', 'Peer'] + ) diff --git a/lms/djangoapps/course_structure_api/v0/serializers.py b/lms/djangoapps/course_structure_api/v0/serializers.py index 877f556779..0ff6b8f1d3 100644 --- a/lms/djangoapps/course_structure_api/v0/serializers.py +++ b/lms/djangoapps/course_structure_api/v0/serializers.py @@ -40,37 +40,3 @@ class CourseSerializer(serializers.Serializer): def get_image_url(self, course): """ Get the course image URL """ return course_image_url(course) - - -class GradingPolicySerializer(serializers.Serializer): - """ Serializer for course grading policy. """ - assignment_type = serializers.CharField(source='type') - count = serializers.IntegerField(source='min_count') - dropped = serializers.IntegerField(source='drop_count') - weight = serializers.FloatField() - - -# pylint: disable=invalid-name -class BlockSerializer(serializers.Serializer): - """ Serializer for course structure block. """ - id = serializers.CharField(source='usage_key') - type = serializers.CharField(source='block_type') - display_name = serializers.CharField() - graded = serializers.BooleanField(default=False) - format = serializers.CharField() - children = serializers.CharField() - - -class CourseStructureSerializer(serializers.Serializer): - """ Serializer for course structure. """ - root = serializers.CharField(source='root') - blocks = serializers.SerializerMethodField('get_blocks') - - def get_blocks(self, structure): - """ Serialize the individual blocks. """ - serialized = {} - - for key, block in structure['blocks'].iteritems(): - serialized[key] = BlockSerializer(block).data - - return serialized diff --git a/lms/djangoapps/course_structure_api/v0/tests.py b/lms/djangoapps/course_structure_api/v0/tests.py index 291b329428..42868abd29 100644 --- a/lms/djangoapps/course_structure_api/v0/tests.py +++ b/lms/djangoapps/course_structure_api/v0/tests.py @@ -9,7 +9,6 @@ from mock import patch, Mock from itertools import product from django.core.urlresolvers import reverse -from django.test.utils import override_settings from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory @@ -336,6 +335,7 @@ class CourseStructureTests(CourseDetailTestMixin, CourseViewTestsMixin, ModuleSt blocks[unicode(xblock.location)] = { u'id': unicode(xblock.location), u'type': xblock.category, + u'parent': None, u'display_name': xblock.display_name, u'format': xblock.format, u'graded': xblock.graded, diff --git a/lms/djangoapps/course_structure_api/v0/views.py b/lms/djangoapps/course_structure_api/v0/views.py index 08f8f56df9..ce2e1ec007 100644 --- a/lms/djangoapps/course_structure_api/v0/views.py +++ b/lms/djangoapps/course_structure_api/v0/views.py @@ -15,14 +15,14 @@ from rest_framework.reverse import reverse from xmodule.modulestore.django import modulestore from opaque_keys.edx.keys import CourseKey -from course_structure_api.v0 import api, serializers -from course_structure_api.v0.errors import CourseNotFoundError, CourseStructureNotAvailableError +from course_structure_api.v0 import serializers from courseware import courses from courseware.access import has_access from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor from openedx.core.lib.api.view_utils import view_course_access, view_auth_classes from openedx.core.lib.api.serializers import PaginationSerializer +from openedx.core.djangoapps.content.course_structures.api.v0 import api, errors from student.roles import CourseInstructorRole, CourseStaffRole from util.module_utils import get_dynamic_descriptor_children @@ -73,7 +73,7 @@ class CourseViewMixin(object): self.course_key = CourseKey.from_string(course_id) self.check_course_permissions(self.request.user, self.course_key) return func(self, *args, **kwargs) - except CourseNotFoundError: + except errors.CourseNotFoundError: raise Http404 return func_wrapper @@ -262,7 +262,7 @@ class CourseStructure(CourseViewMixin, RetrieveAPIView): def get(self, request, **kwargs): try: return Response(api.course_structure(self.course_key)) - except CourseStructureNotAvailableError: + except errors.CourseStructureNotAvailableError: # If we don't have data stored, we will try to regenerate it, so # return a 503 and as them to retry in 2 minutes. return Response(status=503, headers={'Retry-After': '120'}) diff --git a/openedx/core/djangoapps/content/__init__.py b/openedx/core/djangoapps/content/__init__.py index e69de29bb2..762719f3ef 100644 --- a/openedx/core/djangoapps/content/__init__.py +++ b/openedx/core/djangoapps/content/__init__.py @@ -0,0 +1,4 @@ +""" +Setup the signals on startup. +""" +import openedx.core.djangoapps.content.course_structures.signals diff --git a/openedx/core/djangoapps/content/course_structures/api/__init__.py b/openedx/core/djangoapps/content/course_structures/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/content/course_structures/api/v0/__init__.py b/openedx/core/djangoapps/content/course_structures/api/v0/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/course_structure_api/v0/api.py b/openedx/core/djangoapps/content/course_structures/api/v0/api.py similarity index 53% rename from lms/djangoapps/course_structure_api/v0/api.py rename to openedx/core/djangoapps/content/course_structures/api/v0/api.py index 55c884c384..ac46603eff 100644 --- a/lms/djangoapps/course_structure_api/v0/api.py +++ b/openedx/core/djangoapps/content/course_structures/api/v0/api.py @@ -5,11 +5,12 @@ Note: The course list and course detail functionality isn't currently supported of the tricky interactions between DRF and the code. Most of that information is available by accessing the course objects directly. """ - -from course_structure_api.v0 import serializers -from course_structure_api.v0.errors import CourseNotFoundError, CourseStructureNotAvailableError +from collections import OrderedDict +from .serializers import GradingPolicySerializer, CourseStructureSerializer +from .errors import CourseNotFoundError, CourseStructureNotAvailableError from openedx.core.djangoapps.content.course_structures import models, tasks -from courseware import courses +from util.cache import cache +from xmodule.modulestore.django import modulestore def _retrieve_course(course_key): @@ -23,18 +24,24 @@ def _retrieve_course(course_key): CourseNotFoundError """ - try: - return courses.get_course(course_key) - except ValueError: + course = modulestore().get_course(course_key, depth=0) + if course is None: raise CourseNotFoundError + return course -def course_structure(course_key): + +def course_structure(course_key, block_types=None): """ - Retrieves the entire course structure, including information about all the blocks used in the course. + Retrieves the entire course structure, including information about all the blocks used in the + course if `block_types` is None else information about `block_types` will be returned only. + Final serialized information will be cached. Args: course_key: the CourseKey of the course we'd like to retrieve. + block_types: list of required block types. Possible values include sequential, + vertical, html, problem, video, and discussion. The type can also be + the name of a custom type of block used for the course. Returns: The serialized output of the course structure: * root: The ID of the root node of the course structure. @@ -60,15 +67,41 @@ def course_structure(course_key): blocks. Raises: CourseStructureNotAvailableError, CourseNotFoundError + """ course = _retrieve_course(course_key) - try: - requested_course_structure = models.CourseStructure.objects.get(course_id=course.id) - return serializers.CourseStructureSerializer(requested_course_structure.structure).data - except models.CourseStructure.DoesNotExist: - # If we don't have data stored, generate it and return an error. - tasks.update_course_structure.delay(unicode(course_key)) - raise CourseStructureNotAvailableError + + modified_timestamp = models.CourseStructure.objects.filter(course_id=course_key).values('modified') + if modified_timestamp.exists(): + cache_key = 'openedx.content.course_structures.api.v0.api.course_structure.{}.{}.{}'.format( + course_key, modified_timestamp[0]['modified'], '_'.join(block_types or []) + ) + data = cache.get(cache_key) # pylint: disable=maybe-no-member + if data is not None: + return data + + try: + requested_course_structure = models.CourseStructure.objects.get(course_id=course.id) + except models.CourseStructure.DoesNotExist: + pass + else: + structure = requested_course_structure.structure + if block_types is not None: + blocks = requested_course_structure.ordered_blocks + required_blocks = OrderedDict() + for usage_id, block_data in blocks.iteritems(): + if block_data['block_type'] in block_types: + required_blocks[usage_id] = block_data + + structure['blocks'] = required_blocks + + data = CourseStructureSerializer(structure).data + cache.set(cache_key, data, None) # pylint: disable=maybe-no-member + return data + + # If we don't have data stored, generate it and return an error. + tasks.update_course_structure.delay(unicode(course_key)) + raise CourseStructureNotAvailableError def course_grading_policy(course_key): @@ -91,4 +124,4 @@ def course_grading_policy(course_key): final grade. """ course = _retrieve_course(course_key) - return serializers.GradingPolicySerializer(course.raw_grader).data + return GradingPolicySerializer(course.raw_grader).data diff --git a/lms/djangoapps/course_structure_api/v0/errors.py b/openedx/core/djangoapps/content/course_structures/api/v0/errors.py similarity index 100% rename from lms/djangoapps/course_structure_api/v0/errors.py rename to openedx/core/djangoapps/content/course_structures/api/v0/errors.py diff --git a/openedx/core/djangoapps/content/course_structures/api/v0/serializers.py b/openedx/core/djangoapps/content/course_structures/api/v0/serializers.py new file mode 100644 index 0000000000..f440e6cd4a --- /dev/null +++ b/openedx/core/djangoapps/content/course_structures/api/v0/serializers.py @@ -0,0 +1,39 @@ +""" +API Serializers +""" +from rest_framework import serializers + + +class GradingPolicySerializer(serializers.Serializer): + """ Serializer for course grading policy. """ + assignment_type = serializers.CharField(source='type') + count = serializers.IntegerField(source='min_count') + dropped = serializers.IntegerField(source='drop_count') + weight = serializers.FloatField() + + +# pylint: disable=invalid-name +class BlockSerializer(serializers.Serializer): + """ Serializer for course structure block. """ + id = serializers.CharField(source='usage_key') + type = serializers.CharField(source='block_type') + parent = serializers.CharField(source='parent') + display_name = serializers.CharField() + graded = serializers.BooleanField(default=False) + format = serializers.CharField() + children = serializers.CharField() + + +class CourseStructureSerializer(serializers.Serializer): + """ Serializer for course structure. """ + root = serializers.CharField(source='root') + blocks = serializers.SerializerMethodField('get_blocks') + + def get_blocks(self, structure): + """ Serialize the individual blocks. """ + serialized = {} + + for key, block in structure['blocks'].iteritems(): + serialized[key] = BlockSerializer(block).data + + return serialized diff --git a/openedx/core/djangoapps/content/course_structures/api/v0/tests_api.py b/openedx/core/djangoapps/content/course_structures/api/v0/tests_api.py new file mode 100644 index 0000000000..b9cd6d04a1 --- /dev/null +++ b/openedx/core/djangoapps/content/course_structures/api/v0/tests_api.py @@ -0,0 +1,149 @@ +""" +Course Structure api.py tests +""" +from .api import course_structure +from openedx.core.djangoapps.content.course_structures.signals import listen_for_course_publish +from xmodule.modulestore.django import SignalHandler +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +import mock +from django.core import cache + + +class CourseStructureApiTests(ModuleStoreTestCase): + """ + CourseStructure API Tests + """ + MOCK_CACHE = "openedx.core.djangoapps.content.course_structures.api.v0.api.cache" + + def setUp(self): + """ + Test setup + """ + # For some reason, `listen_for_course_publish` is not called when we run + # all (paver test_system -s cms) tests, If we run only run this file then tests run fine. + SignalHandler.course_published.connect(listen_for_course_publish) + + super(CourseStructureApiTests, self).setUp() + self.course = CourseFactory.create() + self.chapter = ItemFactory.create( + parent_location=self.course.location, category='chapter', display_name="Week 1" + ) + self.sequential = ItemFactory.create( + parent_location=self.chapter.location, category='sequential', display_name="Lesson 1" + ) + self.vertical = ItemFactory.create( + parent_location=self.sequential.location, category='vertical', display_name='Subsection 1' + ) + self.video = ItemFactory.create( + parent_location=self.vertical.location, category="video", display_name="My Video" + ) + self.video = ItemFactory.create( + parent_location=self.vertical.location, category="html", display_name="My HTML" + ) + + self.addCleanup(self._disconnect_course_published_event) + + def _disconnect_course_published_event(self): + """ + Disconnect course_published event. + """ + # If we don't disconnect then tests are getting failed in test_crud.py + SignalHandler.course_published.disconnect(listen_for_course_publish) + + def _expected_blocks(self, block_types=None, get_parent=False): + """ + Construct expected blocks. + Arguments: + block_types (list): List of required block types. Possible values include sequential, + vertical, html, problem, video, and discussion. The type can also be + the name of a custom type of block used for the course. + get_parent (bool): If True then add child's parent location else parent is set to None + Returns: + dict: Information about required block types. + """ + blocks = {} + + def add_block(xblock): + """ + Returns expected blocks dict. + """ + children = xblock.get_children() + + if block_types is None or xblock.category in block_types: + + parent = None + if get_parent: + item = xblock.get_parent() + parent = unicode(item.location) if item is not None else None + + blocks[unicode(xblock.location)] = { + u'id': unicode(xblock.location), + u'type': xblock.category, + u'display_name': xblock.display_name, + u'format': xblock.format, + u'graded': xblock.graded, + u'parent': parent, + u'children': [unicode(child.location) for child in children] + } + + for child in children: + add_block(child) + + course = self.store.get_course(self.course.id, depth=None) + add_block(course) + + return blocks + + def test_course_structure_with_no_block_types(self): + """ + Verify that course_structure returns info for entire course. + """ + with mock.patch(self.MOCK_CACHE, cache.get_cache(backend='default')): + with self.assertNumQueries(3): + structure = course_structure(self.course.id) + + expected = { + u'root': unicode(self.course.location), + u'blocks': self._expected_blocks() + } + + self.assertDictEqual(structure, expected) + + with mock.patch(self.MOCK_CACHE, cache.get_cache(backend='default')): + with self.assertNumQueries(2): + course_structure(self.course.id) + + def test_course_structure_with_block_types(self): + """ + Verify that course_structure returns info for required block_types only when specific block_types are requested. + """ + block_types = ['html', 'video'] + + with mock.patch(self.MOCK_CACHE, cache.get_cache(backend='default')): + with self.assertNumQueries(3): + structure = course_structure(self.course.id, block_types=block_types) + + expected = { + u'root': unicode(self.course.location), + u'blocks': self._expected_blocks(block_types=block_types, get_parent=True) + } + + self.assertDictEqual(structure, expected) + + with mock.patch(self.MOCK_CACHE, cache.get_cache(backend='default')): + with self.assertNumQueries(2): + course_structure(self.course.id, block_types=block_types) + + def test_course_structure_with_non_existed_block_types(self): + """ + Verify that course_structure returns empty info for non-existed block_types. + """ + block_types = ['phantom'] + structure = course_structure(self.course.id, block_types=block_types) + expected = { + u'root': unicode(self.course.location), + u'blocks': {} + } + + self.assertDictEqual(structure, expected) diff --git a/openedx/core/djangoapps/content/course_structures/models.py b/openedx/core/djangoapps/content/course_structures/models.py index e0445ecec9..82acf61173 100644 --- a/openedx/core/djangoapps/content/course_structures/models.py +++ b/openedx/core/djangoapps/content/course_structures/models.py @@ -52,7 +52,3 @@ class CourseStructure(TimeStampedModel): for child_node in cur_block['children']: self._traverse_tree(child_node, unordered_structure, ordered_blocks, parent=block) - -# Signals must be imported in a file that is automatically loaded at app startup (e.g. models.py). We import them -# at the end of this file to avoid circular dependencies. -import signals # pylint: disable=unused-import