diff --git a/common/djangoapps/django_comment_common/migrations/0007_discussionsidmapping.py b/common/djangoapps/django_comment_common/migrations/0007_discussionsidmapping.py new file mode 100644 index 0000000000..c41d9cebe3 --- /dev/null +++ b/common/djangoapps/django_comment_common/migrations/0007_discussionsidmapping.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.13 on 2018-06-13 12:10 +from __future__ import unicode_literals + +from django.db import migrations +import jsonfield.fields +import opaque_keys.edx.django.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('django_comment_common', '0006_coursediscussionsettings_discussions_id_map'), + ] + + operations = [ + migrations.CreateModel( + name='DiscussionsIdMapping', + fields=[ + ('course_id', opaque_keys.edx.django.models.CourseKeyField(db_index=True, max_length=255, primary_key=True, serialize=False)), + ('mapping', jsonfield.fields.JSONField(help_text=b'Key/value store mapping discussion IDs to discussion XBlock usage keys.')), + ], + ), + ] diff --git a/common/djangoapps/django_comment_common/models.py b/common/djangoapps/django_comment_common/models.py index 1f7b16a6ec..0d27aa9a07 100644 --- a/common/djangoapps/django_comment_common/models.py +++ b/common/djangoapps/django_comment_common/models.py @@ -204,3 +204,24 @@ class CourseDiscussionSettings(models.Model): def divided_discussions(self, value): """Un-Jsonify the divided_discussions""" self._divided_discussions = json.dumps(value) + + +class DiscussionsIdMapping(models.Model): + """This model is a performance optimization, updated on course publish.""" + course_id = CourseKeyField(db_index=True, primary_key=True, max_length=255) + mapping = JSONField( + help_text="Key/value store mapping discussion IDs to discussion XBlock usage keys.", + ) + + @classmethod + def update_mapping(cls, course_key, discussions_id_map): + """Update the mapping of discussions IDs to XBlock usage key strings.""" + mapping_entry, created = cls.objects.get_or_create( + course_id=course_key, + defaults={ + 'mapping': discussions_id_map, + }, + ) + if not created: + mapping_entry.mapping = discussions_id_map + mapping_entry.save() diff --git a/common/djangoapps/django_comment_common/utils.py b/common/djangoapps/django_comment_common/utils.py index dc810c33bb..30384fbee3 100644 --- a/common/djangoapps/django_comment_common/utils.py +++ b/common/djangoapps/django_comment_common/utils.py @@ -139,8 +139,6 @@ def set_course_discussion_settings(course_key, **kwargs): divided_discussions (list): List of discussion ids. division_scheme (str): `CourseDiscussionSettings.NONE`, `CourseDiscussionSettings.COHORT`, or `CourseDiscussionSettings.ENROLLMENT_TRACK` - discussions_id_map (dict): Dict containing discussion IDs as keys and the associated discussion - XBlock usage keys as values. Returns: A CourseDiscussionSettings object. @@ -149,7 +147,6 @@ def set_course_discussion_settings(course_key, **kwargs): 'division_scheme': basestring, 'always_divide_inline_discussions': bool, 'divided_discussions': list, - 'discussions_id_map': dict, } course_discussion_settings = get_course_discussion_settings(course_key) for field, field_type in fields.items(): diff --git a/lms/djangoapps/discussion/apps.py b/lms/djangoapps/discussion/apps.py index 5e98707ef6..43f7d9d1f6 100644 --- a/lms/djangoapps/discussion/apps.py +++ b/lms/djangoapps/discussion/apps.py @@ -25,9 +25,12 @@ class DiscussionConfig(AppConfig): } }, PluginSettings.CONFIG: { + ProjectType.CMS: { + SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: u'settings.common'}, + }, ProjectType.LMS: { SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: u'settings.common'}, - } + }, } } diff --git a/lms/djangoapps/discussion/tasks.py b/lms/djangoapps/discussion/tasks.py index 00615027f6..b01fe419b4 100644 --- a/lms/djangoapps/discussion/tasks.py +++ b/lms/djangoapps/discussion/tasks.py @@ -13,6 +13,7 @@ from django.contrib.sites.models import Site from celery_utils.logged_task import LoggedTask from django_comment_common.utils import set_course_discussion_settings +from django_comment_common.models import DiscussionsIdMapping from edx_ace import ace from edx_ace.utils import date from edx_ace.recipient import Recipient @@ -48,7 +49,7 @@ def update_discussions_map(context): discussion_block.discussion_id: unicode(discussion_block.location) for discussion_block in discussion_blocks } - set_course_discussion_settings(course_key, discussions_id_map=discussions_id_map) + DiscussionsIdMapping.update_mapping(course_key, discussions_id_map) class ResponseNotification(BaseMessageType): diff --git a/lms/djangoapps/discussion/tests/test_signals.py b/lms/djangoapps/discussion/tests/test_signals.py index d15dbdd04c..d99aee9d5c 100644 --- a/lms/djangoapps/discussion/tests/test_signals.py +++ b/lms/djangoapps/discussion/tests/test_signals.py @@ -70,9 +70,6 @@ class CoursePublishHandlerTestCase(ModuleStoreTestCase): course_key_args = dict(org='org', course='number', run='run') course_key = self.store.make_course_key(**course_key_args) - with self.assertRaises(models.CourseDiscussionSettings.DoesNotExist): - models.CourseDiscussionSettings.objects.get(course_id=course_key) - # create course course = CourseFactory(emit_signals=True, **course_key_args) self.assertEqual(course.id, course_key) @@ -92,5 +89,5 @@ class CoursePublishHandlerTestCase(ModuleStoreTestCase): """ Verifies the discussion ID map for the given course matches the expected value. """ - discussion_settings = models.CourseDiscussionSettings.objects.get(course_id=course_key) - self.assertDictEqual(discussion_settings.discussions_id_map, expected_map) + mapping_entry = models.DiscussionsIdMapping.objects.get(course_id=course_key) + self.assertDictEqual(mapping_entry.mapping, expected_map) diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index a8b86e0f4a..72640ab2a9 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -405,8 +405,8 @@ class ViewsQueryCountTestCase( return inner @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 3, 30), - (ModuleStoreEnum.Type.split, 3, 11, 30), + (ModuleStoreEnum.Type.mongo, 3, 4, 35), + (ModuleStoreEnum.Type.split, 3, 13, 35), ) @ddt.unpack @count_queries @@ -414,8 +414,8 @@ class ViewsQueryCountTestCase( self.create_thread_helper(mock_request) @ddt.data( - (ModuleStoreEnum.Type.mongo, 3, 2, 26), - (ModuleStoreEnum.Type.split, 3, 8, 26), + (ModuleStoreEnum.Type.mongo, 3, 3, 31), + (ModuleStoreEnum.Type.split, 3, 10, 31), ) @ddt.unpack @count_queries diff --git a/setup.py b/setup.py index 3728fa5d83..7051f3b445 100644 --- a/setup.py +++ b/setup.py @@ -79,6 +79,12 @@ setup( ], "cms.djangoapp": [ "ace_common = openedx.core.djangoapps.ace_common.apps:AceCommonConfig", + # Importing an LMS app into the Studio process is not a good + # practice. We're ignoring this for Discussions here because its + # placement in LMS is a historical artifact. The eventual goal is to + # consolidate the multiple discussions-related Django apps and + # either put them in the openedx/ dir, or in another repo entirely. + "discussion = lms.djangoapps.discussion.apps:DiscussionConfig", "plugins = openedx.core.djangoapps.plugins.apps:PluginsConfig", "schedules = openedx.core.djangoapps.schedules.apps:SchedulesConfig", "theming = openedx.core.djangoapps.theming.apps:ThemingConfig",