From b5d72a38d38d32440cda00c9fbbdebca328d396f Mon Sep 17 00:00:00 2001
From: cahrens <christina@edx.org>
Date: Mon, 25 Nov 2013 13:54:45 -0500
Subject: [PATCH] Updates from code review. Includes backing out the
 "as_published" changes I previously put in, as now we are not adding @draft
 to locations converted from locators.

---
 .../contentstore/tests/test_contentstore.py   |  7 +------
 .../contentstore/views/component.py           |  2 +-
 .../xmodule/modulestore/mongo/draft.py        | 10 +++++-----
 .../xmodule/modulestore/xml_exporter.py       | 19 +++++++++----------
 4 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index bc25687342..aaf2cf7660 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -1352,8 +1352,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.assertGreater(len(items), 0)
         for descriptor in items:
             unit_locator = loc_mapper().translate_location(course_id, descriptor.location, False, True)
-            print "Checking {0}....".format(unicode(unit_locator))
-            print descriptor.__class__, descriptor.location
             resp = self.client.get_html(unit_locator.url_reverse('unit'))
             self.assertEqual(resp.status_code, 200)
             # TODO: uncomment when video transcripts no longer require IDs.
@@ -2035,8 +2033,5 @@ def _test_no_locations(test, resp, status_code=200, html=True):
         # it checks that the HTML properly parses. However, it won't find i4x usages
         # in JavaScript blocks.
         content = resp.content
-        num_jump_to_preview = len(re.findall(r"/preview/(\S)*jump_to/i4x", content))
-        num_jump_to_live = len(re.findall(r":8000/(\S)*jump_to/i4x", content))
-        hits = len(re.findall(r"i4x", content)) - num_jump_to_preview - num_jump_to_live
-
+        hits = len(re.findall(r"(?<!jump_to/)i4x://", content))
         test.assertEqual(hits, 0, "i4x found outside of LMS jump-to links")
diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py
index 5c558a83d3..3742c7af20 100644
--- a/cms/djangoapps/contentstore/views/component.py
+++ b/cms/djangoapps/contentstore/views/component.py
@@ -81,7 +81,7 @@ def subsection_handler(request, tag=None, course_id=None, branch=None, version_g
         if len(parent_locs) != 1:
             logging.error(
                 'Multiple (or none) parents have been found for %s',
-                old_location
+                unicode(locator)
             )
 
         # this should blow up if we don't find any parents, which would be erroneous
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
index 6318c5e68f..f7a0250155 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
@@ -81,7 +81,7 @@ class DraftModuleStore(MongoModuleStore):
         try:
             return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(location), depth=depth))
         except ItemNotFoundError:
-            return wrap_draft(super(DraftModuleStore, self).get_item(as_published(location), depth=depth))
+            return wrap_draft(super(DraftModuleStore, self).get_item(location, depth=depth))
 
     def get_instance(self, course_id, location, depth=0):
         """
@@ -169,7 +169,7 @@ class DraftModuleStore(MongoModuleStore):
         try:
             draft_item = self.get_item(location)
             if not getattr(draft_item, 'is_draft', False):
-                self.convert_to_draft(as_published(location))
+                self.convert_to_draft(location)
         except ItemNotFoundError, e:
             if not allow_not_found:
                 raise e
@@ -187,7 +187,7 @@ class DraftModuleStore(MongoModuleStore):
         draft_loc = as_draft(location)
         draft_item = self.get_item(location)
         if not getattr(draft_item, 'is_draft', False):
-            self.convert_to_draft(as_published(location))
+            self.convert_to_draft(location)
 
         return super(DraftModuleStore, self).update_children(draft_loc, children)
 
@@ -203,7 +203,7 @@ class DraftModuleStore(MongoModuleStore):
         draft_item = self.get_item(location)
 
         if not getattr(draft_item, 'is_draft', False):
-            self.convert_to_draft(as_published(location))
+            self.convert_to_draft(location)
 
         if 'is_draft' in metadata:
             del metadata['is_draft']
@@ -262,7 +262,7 @@ class DraftModuleStore(MongoModuleStore):
         """
         Turn the published version into a draft, removing the published version
         """
-        self.convert_to_draft(as_published(location))
+        self.convert_to_draft(location)
         super(DraftModuleStore, self).delete_item(location)
 
     def _query_children_for_cache_children(self, items):
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
index 6e33a54323..2b5efcb87f 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
@@ -97,16 +97,15 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
         if len(draft_verticals) > 0:
             draft_course_dir = export_fs.makeopendir('drafts')
             for draft_vertical in draft_verticals:
-                if getattr(draft_vertical, 'is_draft', False):
-                    parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
-                    # Don't try to export orphaned items.
-                    if len(parent_locs) > 0:
-                        logging.debug('parent_locs = {0}'.format(parent_locs))
-                        draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url()
-                        sequential = modulestore.get_item(Location(parent_locs[0]))
-                        index = sequential.children.index(draft_vertical.location.url())
-                        draft_vertical.xml_attributes['index_in_children_list'] = str(index)
-                        draft_vertical.export_to_xml(draft_course_dir)
+                parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
+                # Don't try to export orphaned items.
+                if len(parent_locs) > 0:
+                    logging.debug('parent_locs = {0}'.format(parent_locs))
+                    draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url()
+                    sequential = modulestore.get_item(Location(parent_locs[0]))
+                    index = sequential.children.index(draft_vertical.location.url())
+                    draft_vertical.xml_attributes['index_in_children_list'] = str(index)
+                    draft_vertical.export_to_xml(draft_course_dir)
 
 
 def export_extra_content(export_fs, modulestore, course_id, course_location, category_type, dirname, file_suffix=''):