Merge pull request #1815 from MITx/fix/cdodge/studio-forum-improvements

Fix/cdodge/studio forum improvements

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -14,6 +14,7 @@ from json import loads
 import traceback
 
 from django.contrib.auth.models import User
+from django.dispatch import Signal
 from contentstore.utils import get_modulestore
 
 from .utils import ModuleStoreTestCase, parse_json
@@ -792,6 +793,45 @@ class ContentStoreTest(ModuleStoreTestCase):
         # make sure we found the item (e.g. it didn't error while loading)
         self.assertTrue(did_load_item)
 
+    def test_forum_id_generation(self):
+        import_from_xml(modulestore(), 'common/test/data/', ['full'])
+        module_store = modulestore('direct')
+        new_component_location = Location('i4x', 'edX', 'full', 'discussion', 'new_component')
+        source_template_location = Location('i4x', 'edx', 'templates', 'discussion', 'Discussion_Tag')
+
+        # crate a new module and add it as a child to a vertical
+        module_store.clone_item(source_template_location, new_component_location)
+
+        new_discussion_item = module_store.get_item(new_component_location)
+
+        self.assertNotEquals(new_discussion_item.discussion_id, '$$GUID$$')
+
+    def test_update_modulestore_signal_did_fire(self):
+
+        import_from_xml(modulestore(), 'common/test/data/', ['full'])
+        module_store = modulestore('direct')
+
+        try:
+            module_store.modulestore_update_signal = Signal(providing_args=['modulestore', 'course_id', 'location'])
+
+            self.got_signal = False
+
+            def _signal_hander(modulestore=None, course_id=None, location=None, **kwargs):
+                self.got_signal = True
+
+            module_store.modulestore_update_signal.connect(_signal_hander)
+
+            new_component_location = Location('i4x', 'edX', 'full', 'html', 'new_component')
+            source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page')
+
+            # crate a new module
+            module_store.clone_item(source_template_location, new_component_location)
+
+        finally:
+            module_store.modulestore_update_signal = None
+
+        self.assertTrue(self.got_signal)
+
     def test_metadata_inheritance(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 

cms/djangoapps/contentstore/utils.py

@@ -11,6 +11,7 @@ DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_ta
 #In order to instantiate an open ended tab automatically, need to have this data
 OPEN_ENDED_PANEL = {"name" : "Open Ended Panel", "type" : "open_ended"}
 
+
 def get_modulestore(location):
     """
     Returns the correct modulestore to use for modifying the specified location

cms/one_time_startup.py

 from dogapi import dog_http_api, dog_stats_api
 from django.conf import settings
 from xmodule.modulestore.django import modulestore
+from django.dispatch import Signal
 from request_cache.middleware import RequestCache
 
-from django.core.cache import get_cache, InvalidCacheBackendError
+from django.core.cache import get_cache
 
 cache = get_cache('mongo_metadata_inheritance')
 for store_name in settings.MODULESTORE:
@@ -11,6 +12,8 @@ for store_name in settings.MODULESTORE:
     store.metadata_inheritance_cache_subsystem = cache
     store.request_cache = RequestCache.get_request_cache()
 
+    modulestore_update_signal = Signal(providing_args=['modulestore', 'course_id', 'location'])
+    store.modulestore_update_signal = modulestore_update_signal
 if hasattr(settings, 'DATADOG_API'):
     dog_http_api.api_key = settings.DATADOG_API
     dog_stats_api.start(api_key=settings.DATADOG_API, statsd=True)

cms/templates/widgets/metadata-only-edit.html

+<%include file="metadata-edit.html" />

common/lib/xmodule/xmodule/discussion_module.py

@@ -3,6 +3,7 @@ from pkg_resources import resource_string, resource_listdir
 
 from xmodule.x_module import XModule
 from xmodule.raw_module import RawDescriptor
+from xmodule.editing_module import MetadataOnlyEditingDescriptor
 from xblock.core import String, Scope
 
 
@@ -28,7 +29,7 @@ class DiscussionModule(DiscussionFields, XModule):
         return self.system.render_template('discussion/_discussion_module.html', context)
 
 
-class DiscussionDescriptor(DiscussionFields, RawDescriptor):
+class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawDescriptor):
     module_class = DiscussionModule
     template_dir_name = "discussion"
 

common/lib/xmodule/xmodule/editing_module.py

@@ -41,6 +41,18 @@ class XMLEditingDescriptor(EditingDescriptor):
     js_module_name = "XMLEditingDescriptor"
 
 
+class MetadataOnlyEditingDescriptor(EditingDescriptor):
+    """
+    Module which only provides an editing interface for the metadata, it does
+    not expose a UI for editing the module data
+    """
+
+    js = {'coffee': [resource_string(__name__, 'js/src/raw/edit/metadata-only.coffee')]}
+    js_module_name = "MetadataOnlyEditingDescriptor"
+
+    mako_template = "widgets/metadata-only-edit.html"
+
+
 class JSONEditingDescriptor(EditingDescriptor):
     """
     Module that provides a raw editing view of its data as XML. It does not perform

common/lib/xmodule/xmodule/js/src/raw/edit/metadata-only.coffee

+class @MetadataOnlyEditingDescriptor extends XModule.Descriptor
+  constructor: (@element) ->
+
+  save: ->
+    data: null

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -252,7 +252,6 @@ class Location(_LocationBase):
     def __repr__(self):
         return "Location%s" % repr(tuple(self))
 
-
     @property
     def course_id(self):
         """Return the ID of the Course that this item belongs to by looking
@@ -414,7 +413,6 @@ class ModuleStore(object):
         return courses
 
 
-
 class ModuleStoreBase(ModuleStore):
     '''
     Implement interface functionality that can be shared.
@@ -425,6 +423,7 @@ class ModuleStoreBase(ModuleStore):
         '''
         self._location_errors = {}    # location -> ErrorLog
         self.metadata_inheritance_cache = None
+        self.modulestore_update_signal = None  # can be set by runtime to route notifications of datastore changes
 
     def _get_errorlog(self, location):
         """

common/lib/xmodule/xmodule/modulestore/mongo.py

@@ -9,6 +9,7 @@ from itertools import repeat
 from path import path
 from datetime import datetime
 from operator import attrgetter
+from uuid import uuid4
 
 from importlib import import_module
 from xmodule.errortracker import null_error_tracker, exc_info_to_str
@@ -30,6 +31,10 @@ log = logging.getLogger(__name__)
 # there is only one revision for each item. Once we start versioning inside the CMS,
 # that assumption will have to change
 
+def get_course_id_no_run(location):
+    '''
+    '''
+    return "/".join([location.org, location.course])
 
 class MongoKeyValueStore(KeyValueStore):
     """
@@ -333,7 +338,7 @@ class MongoModuleStore(ModuleStoreBase):
         '''
         key = metadata_cache_key(location)
         tree = {}
-        
+
         if not force_refresh:
             # see if we are first in the request cache (if present)
             if self.request_cache is not None and key in self.request_cache.data.get('metadata_inheritance', {}):
@@ -348,7 +353,7 @@ class MongoModuleStore(ModuleStoreBase):
         if not tree:
             # if not in subsystem, or we are on force refresh, then we have to compute
             tree = self.compute_metadata_inheritance_tree(location)
-            
+
             # now write out computed tree to caching subsystem (e.g. memcached), if available
             if self.metadata_inheritance_cache_subsystem is not None:
                 self.metadata_inheritance_cache_subsystem.set(key, tree)
@@ -541,8 +546,15 @@ class MongoModuleStore(ModuleStoreBase):
         Clone a new item that is a copy of the item at the location `source`
         and writes it to `location`
         """
+        item = None
         try:
             source_item = self.collection.find_one(location_to_query(source))
+
+            # allow for some programmatically generated substitutions in metadata, e.g. Discussion_id's should be auto-generated
+            for key in source_item['metadata'].keys():
+                if source_item['metadata'][key] == '$$GUID$$':
+                    source_item['metadata'][key] = uuid4().hex
+
             source_item['_id'] = Location(location).dict()
             self.collection.insert(
                 source_item,
@@ -566,12 +578,19 @@ class MongoModuleStore(ModuleStoreBase):
                 course.tabs = existing_tabs
                 self.update_metadata(course.location, course._model_data._kvs._metadata)
 
-            return item
         except pymongo.errors.DuplicateKeyError:
             raise DuplicateItemError(location)
 
         # recompute (and update) the metadata inheritance tree which is cached
         self.refresh_cached_metadata_inheritance_tree(Location(location))
+        self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location))
+
+        return item
+
+    def fire_updated_modulestore_signal(self, course_id, location):
+        if self.modulestore_update_signal is not None:
+            self.modulestore_update_signal.send(self, modulestore=self, course_id=course_id,
+                                                location=location)
 
     def get_course_for_item(self, location, depth=0):
         '''
@@ -643,6 +662,8 @@ class MongoModuleStore(ModuleStoreBase):
         self._update_single_item(location, {'definition.children': children})
         # recompute (and update) the metadata inheritance tree which is cached
         self.refresh_cached_metadata_inheritance_tree(Location(location))
+        # fire signal that we've written to DB
+        self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location))
 
     def update_metadata(self, location, metadata):
         """
@@ -669,6 +690,7 @@ class MongoModuleStore(ModuleStoreBase):
         self._update_single_item(location, {'metadata': metadata})
         # recompute (and update) the metadata inheritance tree which is cached
         self.refresh_cached_metadata_inheritance_tree(loc)
+        self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location))
 
     def delete_item(self, location):
         """
@@ -692,6 +714,7 @@ class MongoModuleStore(ModuleStoreBase):
             safe=self.collection.safe)
         # recompute (and update) the metadata inheritance tree which is cached
         self.refresh_cached_metadata_inheritance_tree(Location(location))
+        self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location))
 
     def get_parent_locations(self, location, course_id):
         '''Find all locations that are the parents of this location in this

common/lib/xmodule/xmodule/raw_module.py

@@ -29,6 +29,6 @@ class RawDescriptor(XmlDescriptor, XMLEditingDescriptor):
             line, offset = err.position
             msg = ("Unable to create xml for problem {loc}. "
                    "Context: '{context}'".format(
-                    context=lines[line - 1][offset - 40:offset + 40],
-                    loc=self.location))
+                   context=lines[line - 1][offset - 40:offset + 40],
+                   loc=self.location))
             raise Exception, msg, sys.exc_info()[2]

common/lib/xmodule/xmodule/templates/discussion/default.yaml

@@ -2,8 +2,8 @@
 metadata:
     display_name: Discussion Tag
     for: Topic-Level Student-Visible Label 
-    id: 6002x_group_discussion_by_this 
+    id: $$GUID$$
     discussion_category: Week 1
 data: |
-  <discussion for="Topic-Level Student-Visible Label" id="6002x_group_discussion_by_this" discussion_category="Week 1" />
+  <discussion />
 children: []

common/lib/xmodule/xmodule/x_module.py

@@ -340,7 +340,9 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
 
     # cdodge: this is a list of metadata names which are 'system' metadata
     # and should not be edited by an end-user
-    system_metadata_fields = ['data_dir', 'published_date', 'published_by', 'is_draft', 'xml_attributes']
+
+    system_metadata_fields = ['data_dir', 'published_date', 'published_by', 'is_draft', 
+        'discussion_id', 'xml_attributes']
 
     # A list of descriptor attributes that must be equal for the descriptors to
     # be equal

lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py

 from django.core.management.base import BaseCommand, CommandError
-from django_comment_client.models import Permission, Role
+from django_comment_client.models import Role
 
 
 class Command(BaseCommand):
@@ -12,18 +12,19 @@ class Command(BaseCommand):
         if len(args) > 1:
             raise CommandError("Too many arguments")
         course_id = args[0]
+
         administrator_role = Role.objects.get_or_create(name="Administrator", course_id=course_id)[0]
         moderator_role = Role.objects.get_or_create(name="Moderator", course_id=course_id)[0]
         community_ta_role = Role.objects.get_or_create(name="Community TA", course_id=course_id)[0]
         student_role = Role.objects.get_or_create(name="Student", course_id=course_id)[0]
 
         for per in ["vote", "update_thread", "follow_thread", "unfollow_thread",
-                       "update_comment", "create_sub_comment", "unvote", "create_thread",
-                       "follow_commentable", "unfollow_commentable", "create_comment", ]:
+                    "update_comment", "create_sub_comment", "unvote", "create_thread",
+                    "follow_commentable", "unfollow_commentable", "create_comment", ]:
             student_role.add_permission(per)
 
         for per in ["edit_content", "delete_thread", "openclose_thread",
-                        "endorse_comment", "delete_comment", "see_all_cohorts"]:
+                    "endorse_comment", "delete_comment", "see_all_cohorts"]:
             moderator_role.add_permission(per)
 
         for per in ["manage_moderator"]:

lms/djangoapps/django_comment_client/utils.py

@@ -146,28 +146,16 @@ def sort_map_entries(category_map):
 
 
 def initialize_discussion_info(course):
-
     global _DISCUSSIONINFO
 
-    # only cache in-memory discussion information for 10 minutes
-    # this is because we need a short-term hack fix for
-    # mongo-backed courseware whereby new discussion modules can be added
-    # without LMS service restart
-
-    if _DISCUSSIONINFO[course.id]:
-        timestamp = _DISCUSSIONINFO[course.id].get('timestamp', datetime.now())
-        age = datetime.now() - timestamp
-        # expire every 5 minutes
-        if age.seconds < 300:
-            return
-
     course_id = course.id
 
     discussion_id_map = {}
     unexpanded_category_map = defaultdict(list)
 
     # get all discussion models within this course_id
-    all_modules = modulestore().get_items(['i4x', course.location.org, course.location.course, 'discussion', None], course_id=course_id)
+    all_modules = modulestore().get_items(['i4x', course.location.org, course.location.course,
+        'discussion', None], course_id=course_id)
 
     for module in all_modules:
         skip_module = False