Merge pull request #18350 from edx/ormsbee/discussion_map_storage_change

Fix forums ID mapping

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.')),
+            ],
+        ),
+    ]

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()

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():

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'},
-            }
+            },
         }
     }
 

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):

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)

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

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",