Merge pull request #2000 from edx/fix/cale/dont-show-template-course

Filter out the template course from the mongo modulestore get_courses function

cms/djangoapps/contentstore/management/commands/update_templates.py

 from xmodule.templates import update_templates
+from xmodule.modulestore.django import modulestore
 from django.core.management.base import BaseCommand
 
 
@@ -6,4 +7,4 @@ class Command(BaseCommand):
     help = 'Imports and updates the Studio component templates from the code pack and put in the DB'
 
     def handle(self, *args, **options):
-        update_templates()
+        update_templates(modulestore('direct'))

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -937,7 +937,7 @@ class TemplateTestCase(ModuleStoreTestCase):
         self.assertIsNotNone(verify_create)
 
         # now run cleanup
-        update_templates()
+        update_templates(modulestore('direct'))
 
         # now try to find dangling template, it should not be in DB any longer
         asserted = False

common/djangoapps/terrain/course_helpers.py

@@ -137,4 +137,4 @@ def clear_courses():
     # $ mongo test_xmodule --eval "db.dropDatabase()"
     _MODULESTORES = {}
     modulestore().collection.drop()
-    update_templates()
+    update_templates(modulestore('direct'))

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

@@ -476,7 +476,15 @@ class MongoModuleStore(ModuleStoreBase):
         '''
         # TODO (vshnayder): Why do I have to specify i4x here?
         course_filter = Location("i4x", category="course")
-        return self.get_items(course_filter)
+        return [
+            course
+            for course
+            in self.get_items(course_filter)
+            if not (
+                course.location.org == 'edx' and
+                course.location.course == 'templates'
+            )
+        ]
 
     def _find_one(self, location):
         '''Look for a given location in the collection.  If revision is not

common/lib/xmodule/xmodule/modulestore/tests/django_utils.py

@@ -42,7 +42,7 @@ class ModuleStoreTestCase(TestCase):
         num_templates = modulestore.collection.find(query).count()
 
         if num_templates < 1:
-            update_templates()
+            update_templates(modulestore)
 
     @classmethod
     def setUpClass(cls):

common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py

@@ -7,6 +7,7 @@ from pprint import pprint
 from xmodule.modulestore import Location
 from xmodule.modulestore.mongo import MongoModuleStore
 from xmodule.modulestore.xml_importer import import_from_xml
+from xmodule.templates import update_templates
 
 from .test_modulestore import check_path_to_location
 from . import DATA_DIR
@@ -45,6 +46,7 @@ class TestMongoModuleStore(object):
         # Explicitly list the courses to load (don't want the big one)
         courses = ['toy', 'simple']
         import_from_xml(store, DATA_DIR, courses)
+        update_templates(store)
         return store
 
     @staticmethod
@@ -103,3 +105,11 @@ class TestMongoModuleStore(object):
     def test_path_to_location(self):
         '''Make sure that path_to_location works'''
         check_path_to_location(self.store)
+
+    def test_get_courses_has_no_templates(self):
+        courses = self.store.get_courses()
+        for course in courses:
+            assert_false(
+                course.location.org == 'edx' and course.location.course == 'templates',
+                '{0} is a template course'.format(course)
+            )

common/lib/xmodule/xmodule/templates.py

@@ -19,7 +19,6 @@ from collections import defaultdict
 from .x_module import XModuleDescriptor
 from .mako_module import MakoDescriptorSystem
 from .modulestore import Location
-from .modulestore.django import modulestore
 
 log = logging.getLogger(__name__)
 
@@ -50,7 +49,7 @@ class TemplateTestSystem(MakoDescriptorSystem):
         )
 
 
-def update_templates():
+def update_templates(modulestore):
     """
     Updates the set of templates in the modulestore with all templates currently
     available from the installed plugins
@@ -58,7 +57,7 @@ def update_templates():
 
     # cdodge: build up a list of all existing templates. This will be used to determine which
     # templates have been removed from disk - and thus we need to remove from the DB
-    templates_to_delete = modulestore('direct').get_items(['i4x', 'edx', 'templates', None, None, None])
+    templates_to_delete = modulestore.get_items(['i4x', 'edx', 'templates', None, None, None])
 
     for category, templates in all_templates().items():
         for template in templates:
@@ -86,9 +85,9 @@ def update_templates():
                 ), exc_info=True)
                 continue
 
-            modulestore('direct').update_item(template_location, template.data)
-            modulestore('direct').update_children(template_location, template.children)
-            modulestore('direct').update_metadata(template_location, template.metadata)
+            modulestore.update_item(template_location, template.data)
+            modulestore.update_children(template_location, template.children)
+            modulestore.update_metadata(template_location, template.metadata)
 
             # remove template from list of templates to delete
             templates_to_delete = [t for t in templates_to_delete if t.location != template_location]
@@ -97,4 +96,4 @@ def update_templates():
     if len(templates_to_delete) > 0:
         logging.debug('deleting dangling templates = {0}'.format(templates_to_delete))
         for template in templates_to_delete:
-            modulestore('direct').delete_item(template.location)
+            modulestore.delete_item(template.location)