From 066b24a6a3f8ea965b5ad305d9fe109545a7ca56 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 29 Jun 2018 00:00:42 -0400 Subject: [PATCH 1/4] CourseStructure: remove from Discussions --- .../django_comment_client/tests/test_utils.py | 15 ++++++++------- lms/djangoapps/django_comment_client/utils.py | 15 +++++++++------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 809641a9fc..24c86b2474 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -25,7 +25,8 @@ from django_comment_client.tests.utils import config_course_discussions, topic_n from django_comment_common.models import ( CourseDiscussionSettings, ForumsConfig, - assign_role + assign_role, + DiscussionsIdMapping, ) from django_comment_common.utils import ( get_course_discussion_settings, @@ -34,7 +35,6 @@ from django_comment_common.utils import ( ) from lms.djangoapps.teams.tests.factories import CourseTeamFactory from lms.lib.comment_client.utils import CommentClientMaintenanceError, perform_request -from openedx.core.djangoapps.content.course_structures.models import CourseStructure from openedx.core.djangoapps.course_groups import cohorts from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory, config_course_cohorts @@ -262,6 +262,7 @@ class CachedDiscussionIdMapTestCase(ModuleStoreTestCase): discussion_target='Beta Testing', visible_to_staff_only=True ) + RequestCache.clear_request_cache() # clear the cache before the last course publish self.bad_discussion = ItemFactory.create( parent_location=self.course.location, category='discussion', @@ -278,14 +279,14 @@ class CachedDiscussionIdMapTestCase(ModuleStoreTestCase): usage_key = utils.get_cached_discussion_key(self.course.id, 'bogus_id') self.assertIsNone(usage_key) - def test_cache_raises_exception_if_course_structure_not_cached(self): - CourseStructure.objects.all().delete() + def test_cache_raises_exception_if_discussion_id_map_not_cached(self): + DiscussionsIdMapping.objects.all().delete() with self.assertRaises(utils.DiscussionIdMapIsNotCached): utils.get_cached_discussion_key(self.course.id, 'test_discussion_id') def test_cache_raises_exception_if_discussion_id_not_cached(self): - cache = CourseStructure.objects.get(course_id=self.course.id) - cache.discussion_id_map_json = None + cache = DiscussionsIdMapping.objects.get(course_id=self.course.id) + cache.mapping = None cache.save() with self.assertRaises(utils.DiscussionIdMapIsNotCached): @@ -313,7 +314,7 @@ class CachedDiscussionIdMapTestCase(ModuleStoreTestCase): self.verify_discussion_metadata() def test_get_discussion_id_map_without_cache(self): - CourseStructure.objects.all().delete() + DiscussionsIdMapping.objects.all().delete() self.verify_discussion_metadata() def test_get_missing_discussion_id_map_from_cache(self): diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index abcbf5fa63..3f61c40f61 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -9,7 +9,7 @@ from django.urls import reverse from django.db import connection from django.http import HttpResponse from pytz import UTC -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locations import i4xEncoder from six import text_type @@ -18,9 +18,8 @@ from courseware.access import has_access from django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY from django_comment_client.permissions import check_permissions_by_view, get_team, has_permission from django_comment_client.settings import MAX_COMMENT_DEPTH -from django_comment_common.models import FORUM_ROLE_STUDENT, CourseDiscussionSettings, Role +from django_comment_common.models import FORUM_ROLE_STUDENT, CourseDiscussionSettings, DiscussionsIdMapping, Role from django_comment_common.utils import get_course_discussion_settings -from openedx.core.djangoapps.content.course_structures.models import CourseStructure from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id, get_cohort_names, is_course_cohorted from openedx.core.djangoapps.request_cache.middleware import request_cached from student.models import get_user_by_username_or_email @@ -164,12 +163,16 @@ def get_cached_discussion_key(course_id, discussion_id): raises a DiscussionIdMapIsNotCached exception. """ try: - mapping = CourseStructure.objects.get(course_id=course_id).discussion_id_map + mapping = DiscussionsIdMapping.objects.get(course_id=course_id).mapping if not mapping: raise DiscussionIdMapIsNotCached() - return mapping.get(discussion_id) - except CourseStructure.DoesNotExist: + usage_key_string = mapping.get(discussion_id) + if usage_key_string: + return UsageKey.from_string(usage_key_string).map_into_course(course_id) + else: + return None + except DiscussionsIdMapping.DoesNotExist: raise DiscussionIdMapIsNotCached() From 4a9063d83b23b7b47fdc6fc2f447cc70634b9662 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 2 Jul 2018 12:36:34 -0400 Subject: [PATCH 2/4] course_structures: remove usages --- .../contentstore/tests/test_import.py | 3 +- .../contentstore/tests/test_libraries.py | 3 +- lms/djangoapps/ccx/tests/test_tasks.py | 29 ------------------- lms/djangoapps/ccx/utils.py | 1 - 4 files changed, 2 insertions(+), 34 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_import.py b/cms/djangoapps/contentstore/tests/test_import.py index 005a59ddd5..eeea1f9d4d 100644 --- a/cms/djangoapps/contentstore/tests/test_import.py +++ b/cms/djangoapps/contentstore/tests/test_import.py @@ -13,7 +13,6 @@ from django.test.client import Client from django.test.utils import override_settings from mock import patch -from openedx.core.djangoapps.content.course_structures.tests import SignalDisconnectTestMixin from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError from xmodule.modulestore import ModuleStoreEnum @@ -30,7 +29,7 @@ TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT @ddt.ddt @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE, SEARCH_ENGINE=None) -class ContentStoreImportTest(SignalDisconnectTestMixin, ModuleStoreTestCase): +class ContentStoreImportTest(ModuleStoreTestCase): """ Tests that rely on the toy and test_import_course courses. NOTE: refactor using CourseFactory so they do not. diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index dfeccb82ae..1d8778e79c 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -12,7 +12,6 @@ from contentstore.views.item import _duplicate_item from contentstore.views.preview import _load_preview_module from contentstore.views.tests.test_library import LIBRARY_REST_URL from course_creators.views import add_user_with_status_granted -from openedx.core.djangoapps.content.course_structures.tests import SignalDisconnectTestMixin from student import auth from student.auth import has_studio_read_access, has_studio_write_access from student.roles import ( @@ -480,7 +479,7 @@ class TestLibraries(LibraryTestCase): @ddt.ddt @patch('django.conf.settings.SEARCH_ENGINE', None) -class TestLibraryAccess(SignalDisconnectTestMixin, LibraryTestCase): +class TestLibraryAccess(LibraryTestCase): """ Test Roles and Permissions related to Content Libraries """ diff --git a/lms/djangoapps/ccx/tests/test_tasks.py b/lms/djangoapps/ccx/tests/test_tasks.py index f42f237ad3..a2f83cb2d6 100644 --- a/lms/djangoapps/ccx/tests/test_tasks.py +++ b/lms/djangoapps/ccx/tests/test_tasks.py @@ -9,7 +9,6 @@ from ccx_keys.locator import CCXLocator from lms.djangoapps.ccx.tasks import send_ccx_course_published from lms.djangoapps.ccx.tests.factories import CcxFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.content.course_structures.models import CourseStructure from student.roles import CourseCcxCoachRole from student.tests.factories import AdminFactory from xmodule.modulestore.django import SignalHandler @@ -74,34 +73,6 @@ class TestSendCCXCoursePublished(ModuleStoreTestCase): self.call_fut(self.course.id) self.assertEqual(receiver.call_count, 3) - def test_course_structure_generated(self): - """ - Check that course structure is generated after course published signal is sent - """ - ccx_structure = { - u"blocks": { - u"ccx-block-v1:edX+999+Run_666+ccx@1+type@course+block@course": { - u"block_type": u"course", - u"graded": False, - u"format": None, - u"usage_key": u"ccx-block-v1:edX+999+Run_666+ccx@1+type@course+block@course", - u"children": [ - ], - u"display_name": u"Run 666" - } - }, - u"root": u"ccx-block-v1:edX+999+Run_666+ccx@1+type@course+block@course" - } - course_key = CCXLocator.from_course_locator(self.course.id, self.ccx.id) - structure = CourseStructure.objects.filter(course_id=course_key) - # no structure exists before signal is called - self.assertEqual(len(structure), 0) - with mock_signal_receiver(SignalHandler.course_published) as receiver: - self.call_fut(self.course.id) - self.assertEqual(receiver.call_count, 3) - structure = CourseStructure.objects.get(course_id=course_key) - self.assertEqual(structure.structure, ccx_structure) - def test_course_overview_cached(self): """ Check that course overview is cached after course published signal is sent diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index f059759fed..8889fb9ad6 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -24,7 +24,6 @@ from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params, from lms.djangoapps.instructor.views.api import _split_input_list from lms.djangoapps.instructor.views.tools import get_student_from_identifier from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.content.course_structures.models import CourseStructure from student.models import CourseEnrollment, CourseEnrollmentException from student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole From afdecc77fdbe52c0926923ecbea49f5ad400a2e0 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 2 Jul 2018 12:40:25 -0400 Subject: [PATCH 3/4] course_structures: remove app code, except migrations and models --- .../content/course_structures/admin.py | 18 -- .../content/course_structures/api/__init__.py | 0 .../course_structures/api/v0/__init__.py | 0 .../content/course_structures/api/v0/api.py | 128 --------- .../course_structures/api/v0/errors.py | 6 - .../course_structures/api/v0/serializers.py | 76 ------ .../course_structures/api/v0/tests_api.py | 153 ----------- .../content/course_structures/apps.py | 8 - .../course_structures/management/__init__.py | 0 .../management/commands/__init__.py | 0 .../commands/generate_course_structure.py | 56 ---- .../content/course_structures/signals.py | 29 --- .../content/course_structures/tasks.py | 100 ------- .../content/course_structures/tests.py | 243 ------------------ 14 files changed, 817 deletions(-) delete mode 100644 openedx/core/djangoapps/content/course_structures/admin.py delete mode 100644 openedx/core/djangoapps/content/course_structures/api/__init__.py delete mode 100644 openedx/core/djangoapps/content/course_structures/api/v0/__init__.py delete mode 100644 openedx/core/djangoapps/content/course_structures/api/v0/api.py delete mode 100644 openedx/core/djangoapps/content/course_structures/api/v0/errors.py delete mode 100644 openedx/core/djangoapps/content/course_structures/api/v0/serializers.py delete mode 100644 openedx/core/djangoapps/content/course_structures/api/v0/tests_api.py delete mode 100644 openedx/core/djangoapps/content/course_structures/management/__init__.py delete mode 100644 openedx/core/djangoapps/content/course_structures/management/commands/__init__.py delete mode 100644 openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py delete mode 100644 openedx/core/djangoapps/content/course_structures/signals.py delete mode 100644 openedx/core/djangoapps/content/course_structures/tasks.py delete mode 100644 openedx/core/djangoapps/content/course_structures/tests.py diff --git a/openedx/core/djangoapps/content/course_structures/admin.py b/openedx/core/djangoapps/content/course_structures/admin.py deleted file mode 100644 index d2b7c41e31..0000000000 --- a/openedx/core/djangoapps/content/course_structures/admin.py +++ /dev/null @@ -1,18 +0,0 @@ -""" -Django Admin model registry for Course Structures sub-application -""" -from django.contrib import admin - -from .models import CourseStructure - - -class CourseStructureAdmin(admin.ModelAdmin): - """ - Django Admin class for managing Course Structures model data - """ - search_fields = ('course_id',) - list_display = ('course_id', 'modified') - ordering = ('course_id', '-modified') - - -admin.site.register(CourseStructure, CourseStructureAdmin) diff --git a/openedx/core/djangoapps/content/course_structures/api/__init__.py b/openedx/core/djangoapps/content/course_structures/api/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/openedx/core/djangoapps/content/course_structures/api/v0/__init__.py b/openedx/core/djangoapps/content/course_structures/api/v0/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/openedx/core/djangoapps/content/course_structures/api/v0/api.py b/openedx/core/djangoapps/content/course_structures/api/v0/api.py deleted file mode 100644 index 1fa2e0b45f..0000000000 --- a/openedx/core/djangoapps/content/course_structures/api/v0/api.py +++ /dev/null @@ -1,128 +0,0 @@ -""" -API implementation of the Course Structure API for Python code. - -Note: The course list and course detail functionality isn't currently supported here because -of the tricky interactions between DRF and the code. -Most of that information is available by accessing the course objects directly. -""" -from collections import OrderedDict -from openedx.core.lib.exceptions import CourseNotFoundError -from .serializers import GradingPolicySerializer, CourseStructureSerializer -from .errors import CourseStructureNotAvailableError -from openedx.core.djangoapps.content.course_structures import models, tasks -from util.cache import cache -from xmodule.modulestore.django import modulestore - - -def _retrieve_course(course_key): - """Retrieves the course for the given course key. - - Args: - course_key: The CourseKey for the course we'd like to retrieve. - Returns: - the course that matches the CourseKey - Raises: - CourseNotFoundError - - """ - course = modulestore().get_course(course_key, depth=0) - if course is None: - raise CourseNotFoundError - - return course - - -def course_structure(course_key, block_types=None): - """ - 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. - - * blocks: A dictionary that maps block IDs to a collection of - information about each block. Each block contains the following - fields. - - * id: The ID of the block. - - * type: The type of block. 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. - - * display_name: The display name configured for the block. - - * graded: Whether or not the sequential or problem is graded. The - value is true or false. - - * format: The assignment type. - - * children: If the block has child blocks, a list of IDs of the child - blocks. - Raises: - CourseStructureNotAvailableError, CourseNotFoundError - - """ - course = _retrieve_course(course_key) - - 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): - """ - Retrieves the course grading policy. - - Args: - course_key: CourseKey the corresponds to the course we'd like to know grading policy information about. - Returns: - The serialized version of the course grading policy containing the following information: - * assignment_type: The type of the assignment, as configured by course - staff. For example, course staff might make the assignment types Homework, - Quiz, and Exam. - - * count: The number of assignments of the type. - - * dropped: Number of assignments of the type that are dropped. - - * weight: The weight, or effect, of the assignment type on the learner's - final grade. - """ - course = _retrieve_course(course_key) - return GradingPolicySerializer(course.raw_grader, many=True).data diff --git a/openedx/core/djangoapps/content/course_structures/api/v0/errors.py b/openedx/core/djangoapps/content/course_structures/api/v0/errors.py deleted file mode 100644 index 378a8ca975..0000000000 --- a/openedx/core/djangoapps/content/course_structures/api/v0/errors.py +++ /dev/null @@ -1,6 +0,0 @@ -""" Errors used by the Course Structure API. """ - - -class CourseStructureNotAvailableError(Exception): - """ The course structure still needs to be generated. """ - pass diff --git a/openedx/core/djangoapps/content/course_structures/api/v0/serializers.py b/openedx/core/djangoapps/content/course_structures/api/v0/serializers.py deleted file mode 100644 index 881be80437..0000000000 --- a/openedx/core/djangoapps/content/course_structures/api/v0/serializers.py +++ /dev/null @@ -1,76 +0,0 @@ -""" -API Serializers -""" -from collections import defaultdict - -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() - - def to_representation(self, obj): - """ - Return a representation of the grading policy. - """ - # Backwards compatibility with the behavior of DRF v2. - # When the grader dictionary was missing keys, DRF v2 would default to None; - # DRF v3 unhelpfully raises an exception. - return dict( - super(GradingPolicySerializer, self).to_representation( - defaultdict(lambda: None, obj) - ) - ) - - -# 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(required=False) - display_name = serializers.CharField() - graded = serializers.BooleanField(default=False) - format = serializers.CharField() - children = serializers.CharField() - - def to_representation(self, obj): - """ - Return a representation of the block. - - NOTE: this method maintains backwards compatibility with the behavior - of Django Rest Framework v2. - """ - data = super(BlockSerializer, self).to_representation(obj) - - # Backwards compatibility with the behavior of DRF v2 - # Include a NULL value for "parent" in the representation - # (instead of excluding the key entirely) - if obj.get("parent") is None: - data["parent"] = None - - # Backwards compatibility with the behavior of DRF v2 - # Leave the children list as a list instead of serializing - # it to a string. - data["children"] = obj["children"] - - return data - - -class CourseStructureSerializer(serializers.Serializer): - """ Serializer for course structure. """ - root = serializers.CharField() - blocks = serializers.SerializerMethodField() - - 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 deleted file mode 100644 index d61d7b87e1..0000000000 --- a/openedx/core/djangoapps/content/course_structures/api/v0/tests_api.py +++ /dev/null @@ -1,153 +0,0 @@ -""" -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" - - ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache'] - - ENABLED_SIGNALS = ['course_published'] - - 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.caches['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.caches['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.caches['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.caches['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/apps.py b/openedx/core/djangoapps/content/course_structures/apps.py index 51ca56a572..a41d0b9234 100644 --- a/openedx/core/djangoapps/content/course_structures/apps.py +++ b/openedx/core/djangoapps/content/course_structures/apps.py @@ -9,11 +9,3 @@ class CourseStructuresConfig(AppConfig): Custom AppConfig for openedx.core.djangoapps.content.course_structures """ name = u'openedx.core.djangoapps.content.course_structures' - - def ready(self): - """ - Define tasks to perform at app loading time: - - * Connect signal handlers - """ - from . import signals # pylint: disable=unused-variable diff --git a/openedx/core/djangoapps/content/course_structures/management/__init__.py b/openedx/core/djangoapps/content/course_structures/management/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/openedx/core/djangoapps/content/course_structures/management/commands/__init__.py b/openedx/core/djangoapps/content/course_structures/management/commands/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py b/openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py deleted file mode 100644 index 2e09ecf154..0000000000 --- a/openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py +++ /dev/null @@ -1,56 +0,0 @@ -""" -Django Management Command: Generate Course Structure -Generates and stores course structure information for one or more courses. -""" -import logging - -from django.core.management.base import BaseCommand -from opaque_keys.edx.keys import CourseKey -from six import text_type - -from openedx.core.djangoapps.content.course_structures.tasks import update_course_structure -from xmodule.modulestore.django import modulestore - -log = logging.getLogger(__name__) - - -class Command(BaseCommand): - """ - Generates and stores course structure information for one or more courses. - """ - help = 'Generates and stores course structure for one or more courses.' - - def add_arguments(self, parser): - parser.add_argument('course_id', nargs='*') - parser.add_argument('--all', - action='store_true', - help='Generate structures for all courses.') - - def handle(self, *args, **options): - """ - Perform the course structure generation workflow - """ - if options['all']: - course_keys = [course.id for course in modulestore().get_courses()] - else: - course_keys = [CourseKey.from_string(arg) for arg in options['course_id']] - - if not course_keys: - log.fatal('No courses specified.') - return - - log.info('Generating course structures for %d courses.', len(course_keys)) - log.debug('Generating course structure(s) for the following courses: %s', course_keys) - - for course_key in course_keys: - try: - # Run the update task synchronously so that we know when all course structures have been updated. - # TODO Future improvement: Use .delay(), add return value to ResultSet, and wait for execution of - # all tasks using ResultSet.join(). I (clintonb) am opting not to make this improvement right now - # as I do not have time to test it fully. - update_course_structure.apply(args=[text_type(course_key)]) - except Exception as ex: - log.exception('An error occurred while generating course structure for %s: %s', - text_type(course_key), text_type(ex)) - - log.info('Finished generating course structures.') diff --git a/openedx/core/djangoapps/content/course_structures/signals.py b/openedx/core/djangoapps/content/course_structures/signals.py deleted file mode 100644 index 0770d573b4..0000000000 --- a/openedx/core/djangoapps/content/course_structures/signals.py +++ /dev/null @@ -1,29 +0,0 @@ -""" -Django Signals classes and functions for the Course Structure application -""" -from django.dispatch.dispatcher import receiver - -from xmodule.modulestore.django import SignalHandler - -from .models import CourseStructure - - -@receiver(SignalHandler.course_published) -def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument - """ - Course Structure application receiver for the course_published signal - """ - # Import tasks here to avoid a circular import. - from .tasks import update_course_structure - - # Delete the existing discussion id map cache to avoid inconsistencies - try: - structure = CourseStructure.objects.get(course_id=course_key) - structure.discussion_id_map_json = None - structure.save() - except CourseStructure.DoesNotExist: - pass - - # Note: The countdown=0 kwarg is set to to ensure the method below does not attempt to access the course - # before the signal emitter has finished all operations. This is also necessary to ensure all tests pass. - update_course_structure.apply_async([unicode(course_key)], countdown=0) diff --git a/openedx/core/djangoapps/content/course_structures/tasks.py b/openedx/core/djangoapps/content/course_structures/tasks.py deleted file mode 100644 index bccf529c55..0000000000 --- a/openedx/core/djangoapps/content/course_structures/tasks.py +++ /dev/null @@ -1,100 +0,0 @@ -""" -Asynchronous tasks related to the Course Structure sub-application -""" -import json -import logging - -from celery.task import task -from opaque_keys.edx.keys import CourseKey -from six import text_type - -from xmodule.modulestore.django import modulestore - - -log = logging.getLogger('edx.celery.task') - - -def _generate_course_structure(course_key): - """ - Generates a course structure dictionary for the specified course. - """ - with modulestore().bulk_operations(course_key): - course = modulestore().get_course(course_key, depth=None) - blocks_stack = [course] - blocks_dict = {} - discussions = {} - while blocks_stack: - curr_block = blocks_stack.pop() - children = curr_block.get_children() if curr_block.has_children else [] - key = unicode(curr_block.scope_ids.usage_id) - block = { - "usage_key": key, - "block_type": curr_block.category, - "display_name": curr_block.display_name, - "children": [unicode(child.scope_ids.usage_id) for child in children] - } - - if (curr_block.category == 'discussion' and - hasattr(curr_block, 'discussion_id') and - curr_block.discussion_id): - discussions[curr_block.discussion_id] = unicode(curr_block.scope_ids.usage_id) - - # Retrieve these attributes separately so that we can fail gracefully - # if the block doesn't have the attribute. - attrs = (('graded', False), ('format', None)) - for attr, default in attrs: - if hasattr(curr_block, attr): - block[attr] = getattr(curr_block, attr, default) - else: - log.warning('Failed to retrieve %s attribute of block %s. Defaulting to %s.', attr, key, default) - block[attr] = default - - blocks_dict[key] = block - - # Add this blocks children to the stack so that we can traverse them as well. - blocks_stack.extend(children) - return { - 'structure': { - "root": unicode(course.scope_ids.usage_id), - "blocks": blocks_dict - }, - 'discussion_id_map': discussions - } - - -@task(name=u'openedx.core.djangoapps.content.course_structures.tasks.update_course_structure') -def update_course_structure(course_key): - """ - Regenerates and updates the course structure (in the database) for the specified course. - """ - # Import here to avoid circular import. - from .models import CourseStructure - - # Ideally we'd like to accept a CourseLocator; however, CourseLocator is not JSON-serializable (by default) so - # Celery's delayed tasks fail to start. For this reason, callers should pass the course key as a Unicode string. - if not isinstance(course_key, basestring): - raise ValueError('course_key must be a string. {} is not acceptable.'.format(type(course_key))) - - course_key = CourseKey.from_string(course_key) - - try: - structure = _generate_course_structure(course_key) - except Exception as ex: - log.exception('An error occurred while generating course structure: %s', text_type(ex)) - raise - - structure_json = json.dumps(structure['structure']) - discussion_id_map_json = json.dumps(structure['discussion_id_map']) - - structure_model, created = CourseStructure.objects.get_or_create( - course_id=course_key, - defaults={ - 'structure_json': structure_json, - 'discussion_id_map_json': discussion_id_map_json - } - ) - - if not created: - structure_model.structure_json = structure_json - structure_model.discussion_id_map_json = discussion_id_map_json - structure_model.save() diff --git a/openedx/core/djangoapps/content/course_structures/tests.py b/openedx/core/djangoapps/content/course_structures/tests.py deleted file mode 100644 index 3c1bef4c19..0000000000 --- a/openedx/core/djangoapps/content/course_structures/tests.py +++ /dev/null @@ -1,243 +0,0 @@ -""" -Course Structure Content sub-application test cases -""" -import json -from nose.plugins.attrib import attr -from opaque_keys.edx.django.models import UsageKey - -from xmodule.modulestore.django import SignalHandler -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from openedx.core.djangoapps.content.course_structures.models import CourseStructure -from openedx.core.djangoapps.content.course_structures.signals import listen_for_course_publish -from openedx.core.djangoapps.content.course_structures.tasks import _generate_course_structure, update_course_structure - - -class SignalDisconnectTestMixin(object): - """ - Mixin for tests to disable calls to signals.listen_for_course_publish when the course_published signal is fired. - """ - - def setUp(self): - super(SignalDisconnectTestMixin, self).setUp() - SignalHandler.course_published.disconnect(listen_for_course_publish) - - def tearDown(self): - SignalHandler.course_published.connect(listen_for_course_publish) - super(SignalDisconnectTestMixin, self).tearDown() - - -@attr(shard=2) -class CourseStructureTaskTests(ModuleStoreTestCase): - """ - Test cases covering Course Structure task-related workflows - """ - def setUp(self, **kwargs): - super(CourseStructureTaskTests, self).setUp() - self.course = CourseFactory.create(org='TestX', course='TS101', run='T1') - self.section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') - self.discussion_xblock_1 = ItemFactory.create( - parent=self.course, - category='discussion', - discussion_id='test_discussion_id_1' - ) - self.discussion_xblock_2 = ItemFactory.create( - parent=self.course, - category='discussion', - discussion_id='test_discussion_id_2' - ) - CourseStructure.objects.all().delete() - - def test_generate_course_structure(self): - blocks = {} - - def add_block(block): - """ - Inserts new child XBlocks into the existing course tree - """ - children = block.get_children() if block.has_children else [] - - blocks[unicode(block.location)] = { - "usage_key": unicode(block.location), - "block_type": block.category, - "display_name": block.display_name, - "graded": block.graded, - "format": block.format, - "children": [unicode(child.location) for child in children] - } - - for child in children: - add_block(child) - - add_block(self.course) - - expected = { - 'root': unicode(self.course.location), - 'blocks': blocks - } - - self.maxDiff = None - actual = _generate_course_structure(self.course.id) - self.assertDictEqual(actual['structure'], expected) - - def test_structure_json(self): - """ - Although stored as compressed data, CourseStructure.structure_json should always return the uncompressed string. - """ - course_id = 'a/b/c' - structure = { - 'root': course_id, - 'blocks': { - course_id: { - 'id': course_id - } - } - } - structure_json = json.dumps(structure) - structure = CourseStructure.objects.create(course_id=self.course.id, structure_json=structure_json) - self.assertEqual(structure.structure_json, structure_json) - - # Reload the data to ensure the init signal is fired to decompress the data. - cs = CourseStructure.objects.get(course_id=self.course.id) - self.assertEqual(cs.structure_json, structure_json) - - def test_structure(self): - """ - CourseStructure.structure should return the uncompressed, JSON-parsed course structure. - """ - structure = { - 'root': 'a/b/c', - 'blocks': { - 'a/b/c': { - 'id': 'a/b/c' - } - } - } - structure_json = json.dumps(structure) - cs = CourseStructure.objects.create(course_id=self.course.id, structure_json=structure_json) - self.assertDictEqual(cs.structure, structure) - - def test_ordered_blocks(self): - structure = { - 'root': 'a/b/c', - 'blocks': { - 'a/b/c': { - 'id': 'a/b/c', - 'children': [ - 'g/h/i' - ] - }, - 'd/e/f': { - 'id': 'd/e/f', - 'children': [] - }, - 'g/h/i': { - 'id': 'h/j/k', - 'children': [ - 'j/k/l', - 'd/e/f' - ] - }, - 'j/k/l': { - 'id': 'j/k/l', - 'children': [] - } - } - } - in_order_blocks = ['a/b/c', 'g/h/i', 'j/k/l', 'd/e/f'] - structure_json = json.dumps(structure) - retrieved_course_structure = CourseStructure.objects.create( - course_id=self.course.id, structure_json=structure_json - ) - - self.assertEqual(retrieved_course_structure.ordered_blocks.keys(), in_order_blocks) - - def test_block_with_missing_fields(self): - """ - The generator should continue to operate on blocks/XModule that do not have graded or format fields. - """ - # TODO In the future, test logging using testfixtures.LogCapture - # (https://pythonhosted.org/testfixtures/logging.html). Talk to TestEng before adding that library. - category = 'peergrading' - display_name = 'Testing Module' - module = ItemFactory.create(parent=self.section, category=category, display_name=display_name) - structure = _generate_course_structure(self.course.id) - - usage_key = unicode(module.location) - actual = structure['structure']['blocks'][usage_key] - expected = { - "usage_key": usage_key, - "block_type": category, - "display_name": display_name, - "graded": False, - "format": None, - "children": [] - } - self.assertEqual(actual, expected) - - def test_generate_discussion_id_map(self): - id_map = {} - - def add_block(block): - """Adds the given block and all of its children to the expected discussion id map""" - children = block.get_children() if block.has_children else [] - - if block.category == 'discussion': - id_map[block.discussion_id] = unicode(block.location) - - for child in children: - add_block(child) - - add_block(self.course) - - actual = _generate_course_structure(self.course.id) - self.assertEqual(actual['discussion_id_map'], id_map) - - def test_discussion_id_map_json(self): - id_map = { - 'discussion_id_1': 'module_location_1', - 'discussion_id_2': 'module_location_2' - } - id_map_json = json.dumps(id_map) - structure = CourseStructure.objects.create(course_id=self.course.id, discussion_id_map_json=id_map_json) - self.assertEqual(structure.discussion_id_map_json, id_map_json) - - structure = CourseStructure.objects.get(course_id=self.course.id) - self.assertEqual(structure.discussion_id_map_json, id_map_json) - - def test_discussion_id_map(self): - id_map = { - 'discussion_id_1': 'block-v1:TestX+TS101+T1+type@discussion+block@b141953dff414921a715da37eb14ecdc', - 'discussion_id_2': 'i4x://TestX/TS101/discussion/466f474fa4d045a8b7bde1b911e095ca' - } - id_map_json = json.dumps(id_map) - structure = CourseStructure.objects.create(course_id=self.course.id, discussion_id_map_json=id_map_json) - expected_id_map = { - key: UsageKey.from_string(value).map_into_course(self.course.id) - for key, value in id_map.iteritems() - } - self.assertEqual(structure.discussion_id_map, expected_id_map) - - def test_discussion_id_map_missing(self): - structure = CourseStructure.objects.create(course_id=self.course.id) - self.assertIsNone(structure.discussion_id_map) - - def test_update_course_structure(self): - """ - Test the actual task that orchestrates data generation and updating the database. - """ - # Method requires string input - course_id = self.course.id - self.assertRaises(ValueError, update_course_structure, course_id) - - # Ensure a CourseStructure object is created - expected_structure = _generate_course_structure(course_id) - update_course_structure(unicode(course_id)) - structure = CourseStructure.objects.get(course_id=course_id) - self.assertEqual(structure.course_id, course_id) - self.assertEqual(structure.structure, expected_structure['structure']) - self.assertEqual(structure.discussion_id_map.keys(), expected_structure['discussion_id_map'].keys()) - self.assertEqual( - [unicode(value) for value in structure.discussion_id_map.values()], - expected_structure['discussion_id_map'].values() - ) From 77bf86fb7305df2de45e1f8c0198ff04c561b512 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 3 Jul 2018 06:47:02 -0400 Subject: [PATCH 4/4] Fix unclearly related test failure --- lms/djangoapps/mobile_api/users/tests.py | 88 +++++++++++++----------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 16fd927fec..e5e81b9756 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -202,13 +202,49 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.assertEqual(response.data[0]['course']['start_type'], expected_type) self.assertEqual(response.data[0]['course']['start_display'], expected_display) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) - def test_no_certificate(self): + @patch.dict(settings.FEATURES, {"ENABLE_DISCUSSION_SERVICE": True, 'ENABLE_MKTG_SITE': True}) + def test_discussion_url(self): self.login_and_enroll() response = self.api_response() - certificate_data = response.data[0]['certificate'] - self.assertDictEqual(certificate_data, {}) + response_discussion_url = response.data[0]['course']['discussion_url'] + self.assertIn('/api/discussion/v1/courses/{}'.format(self.course.id), response_discussion_url) + + def test_org_query(self): + self.login() + + # Create list of courses with various organizations + courses = [ + CourseFactory.create(org='edX', mobile_available=True), + CourseFactory.create(org='edX', mobile_available=True), + CourseFactory.create(org='edX', mobile_available=True, visible_to_staff_only=True), + CourseFactory.create(org='Proversity.org', mobile_available=True), + CourseFactory.create(org='MITx', mobile_available=True), + CourseFactory.create(org='HarvardX', mobile_available=True), + ] + + # Enroll in all the courses + for course in courses: + self.enroll(course.id) + + response = self.api_response(data={'org': 'edX'}) + + # Test for 3 expected courses + self.assertEqual(len(response.data), 3) + + # Verify only edX courses are returned + for entry in response.data: + self.assertEqual(entry['course']['org'], 'edX') + + +@attr(shard=9) +@override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) +class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): + """ + Tests for /api/mobile/v0.5/users//course_enrollments/ + """ + REVERSE_INFO = {'name': 'courseenrollment-detail', 'params': ['username']} + ENABLED_SIGNALS = ['course_published'] def verify_pdf_certificate(self): """ @@ -230,10 +266,18 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest certificate_data = response.data[0]['certificate'] self.assertEquals(certificate_data['url'], certificate_url) + @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) + def test_no_certificate(self): + self.login_and_enroll() + + response = self.api_response() + certificate_data = response.data[0]['certificate'] + self.assertDictEqual(certificate_data, {}) + @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': False, 'ENABLE_MKTG_SITE': True}) def test_pdf_certificate_with_html_cert_disabled(self): """ - Tests PDF certificates with CERTIFICATES_HTML_VIEW set to False. + Tests PDF certificates with CERTIFICATES_HTML_VIEW set to True. """ self.verify_pdf_certificate() @@ -280,40 +324,6 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest ) ) - @patch.dict(settings.FEATURES, {"ENABLE_DISCUSSION_SERVICE": True, 'ENABLE_MKTG_SITE': True}) - def test_discussion_url(self): - self.login_and_enroll() - - response = self.api_response() - response_discussion_url = response.data[0]['course']['discussion_url'] # pylint: disable=E1101 - self.assertIn('/api/discussion/v1/courses/{}'.format(self.course.id), response_discussion_url) - - def test_org_query(self): - self.login() - - # Create list of courses with various organizations - courses = [ - CourseFactory.create(org='edX', mobile_available=True), - CourseFactory.create(org='edX', mobile_available=True), - CourseFactory.create(org='edX', mobile_available=True, visible_to_staff_only=True), - CourseFactory.create(org='Proversity.org', mobile_available=True), - CourseFactory.create(org='MITx', mobile_available=True), - CourseFactory.create(org='HarvardX', mobile_available=True), - ] - - # Enroll in all the courses - for course in courses: - self.enroll(course.id) - - response = self.api_response(data={'org': 'edX'}) - - # Test for 3 expected courses - self.assertEqual(len(response.data), 3) - - # Verify only edX courses are returned - for entry in response.data: - self.assertEqual(entry['course']['org'], 'edX') - @attr(shard=9) class CourseStatusAPITestCase(MobileAPITestCase):