compute_publish_state -> has_published_version

LMS-11213

cms/djangoapps/contentstore/tests/utils.py

@@ -9,17 +9,14 @@ from django.test.client import Client
 from django.contrib.auth.models import User
 
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore import PublishState, ModuleStoreEnum
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
-from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES
 from xmodule.modulestore.xml_importer import import_from_xml
 from student.models import Registration
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
 from contentstore.utils import reverse_url
-from xmodule.modulestore.mongo.draft import DraftModuleStore
-from xblock.fields import Scope
 from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore
 
 
@@ -151,16 +148,16 @@ class CourseTestCase(ModuleStoreTestCase):
         # create a Draft vertical
         vertical = self.store.get_item(course_id.make_usage_key('vertical', self.TEST_VERTICAL), depth=1)
         draft_vertical = self.store.convert_to_draft(vertical.location, self.user.id)
-        self.assertEqual(self.store.compute_publish_state(draft_vertical), PublishState.draft)
+        self.assertTrue(self.store.has_published_version(draft_vertical))
 
         # create a Private (draft only) vertical
         private_vertical = self.store.create_item(self.user.id, course_id, 'vertical', self.PRIVATE_VERTICAL)
-        self.assertEqual(self.store.compute_publish_state(private_vertical), PublishState.private)
+        self.assertFalse(self.store.has_published_version(private_vertical))
 
         # create a Published (no draft) vertical
         public_vertical = self.store.create_item(self.user.id, course_id, 'vertical', self.PUBLISHED_VERTICAL)
         public_vertical = self.store.publish(public_vertical.location, self.user.id)
-        self.assertEqual(self.store.compute_publish_state(public_vertical), PublishState.public)
+        self.assertTrue(self.store.has_published_version(public_vertical))
 
         # add the new private and new public as children of the sequential
         sequential = self.store.get_item(course_id.make_usage_key('sequential', self.SEQUENTIAL))
@@ -197,11 +194,7 @@ class CourseTestCase(ModuleStoreTestCase):
 
         def verify_item_publish_state(item, publish_state):
             """Verifies the publish state of the item is as expected."""
-            if publish_state in (PublishState.private, PublishState.draft):
-                self.assertTrue(getattr(item, 'is_draft', False))
-            else:
-                self.assertFalse(getattr(item, 'is_draft', False))
-            self.assertEqual(self.store.compute_publish_state(item), publish_state)
+            self.assertEqual(self.store.has_published_version(item), publish_state)
 
         def get_and_verify_publish_state(item_type, item_name, publish_state):
             """Gets the given item from the store and verifies the publish state of the item is as expected."""
@@ -210,18 +203,18 @@ class CourseTestCase(ModuleStoreTestCase):
             return item
 
         # verify that the draft vertical is draft
-        vertical = get_and_verify_publish_state('vertical', self.TEST_VERTICAL, PublishState.draft)
+        vertical = get_and_verify_publish_state('vertical', self.TEST_VERTICAL, True)
         for child in vertical.get_children():
-            verify_item_publish_state(child, PublishState.draft)
+            verify_item_publish_state(child, True)
 
         # make sure that we don't have a sequential that is not in draft mode
-        sequential = get_and_verify_publish_state('sequential', self.SEQUENTIAL, PublishState.public)
+        sequential = get_and_verify_publish_state('sequential', self.SEQUENTIAL, True)
 
         # verify that we have the private vertical
-        private_vertical = get_and_verify_publish_state('vertical', self.PRIVATE_VERTICAL, PublishState.private)
+        private_vertical = get_and_verify_publish_state('vertical', self.PRIVATE_VERTICAL, False)
 
         # verify that we have the public vertical
-        public_vertical = get_and_verify_publish_state('vertical', self.PUBLISHED_VERTICAL, PublishState.public)
+        public_vertical = get_and_verify_publish_state('vertical', self.PUBLISHED_VERTICAL, True)
 
         # verify verticals are children of sequential
         for vert in [vertical, private_vertical, public_vertical]:
@@ -266,22 +259,11 @@ class CourseTestCase(ModuleStoreTestCase):
                 course2_item_loc = course2_item_loc.replace(name=new_name)
             course2_item = self.store.get_item(course2_item_loc)
 
-            try:
-                # compare published state
-                self.assertEqual(
-                    self.store.compute_publish_state(course1_item),
-                    self.store.compute_publish_state(course2_item)
-                )
-            except AssertionError:
-                c1_state = self.compute_real_state(course1_item)
-                c2_state = self.compute_real_state(course2_item)
-                self.assertEqual(
-                    c1_state,
-                    c2_state,
-                    "Publish states not equal: course item {} in state {} != course item {} in state {}".format(
-                        course1_item_loc, c1_state, course2_item.location, c2_state
-                    )
-                )
+            # compare published state
+            self.assertEqual(
+                self.store.has_published_version(course1_item),
+                self.store.has_published_version(course2_item)
+            )
 
             # compare data
             self.assertEqual(hasattr(course1_item, 'data'), hasattr(course2_item, 'data'))
@@ -332,37 +314,6 @@ class CourseTestCase(ModuleStoreTestCase):
             else:
                 self.assertEqual(value, course2_asset_attrs[key])
 
-    def compute_real_state(self, item):
-        """
-        In draft mongo, compute_published_state can return draft when the draft == published, but in split,
-        it'll return public in that case
-        """
-        supposed_state = self.store.compute_publish_state(item)
-        if supposed_state == PublishState.draft and isinstance(item.runtime.modulestore, DraftModuleStore):
-            # see if the draft differs from the published
-            published = self.store.get_item(item.location, revision=ModuleStoreEnum.RevisionOption.published_only)
-            if item.get_explicitly_set_fields_by_scope() != published.get_explicitly_set_fields_by_scope():
-                # checking content: if published differs from item, return draft
-                return supposed_state
-            if item.get_explicitly_set_fields_by_scope(Scope.settings) != published.get_explicitly_set_fields_by_scope(Scope.settings):
-                # checking settings: if published differs from item, return draft
-                return supposed_state
-            if item.has_children and item.children != published.children:
-                # checking children: if published differs from item, return draft
-                return supposed_state
-            # published == item in all respects, so return public
-            return PublishState.public
-        elif supposed_state == PublishState.public and item.location.category in DIRECT_ONLY_CATEGORIES:
-            if not all([
-                self.store.has_item(child_loc, revision=ModuleStoreEnum.RevisionOption.draft_only)
-                for child_loc in item.children
-            ]):
-                return PublishState.draft
-            else:
-                return supposed_state
-        else:
-            return supposed_state
-
 
 def get_url(handler_name, key_value, key_name='usage_key_string', kwargs=None):
     """

cms/djangoapps/contentstore/views/component.py

@@ -11,7 +11,6 @@ from django.conf import settings
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from edxmako.shortcuts import render_to_response
 
-from xmodule.modulestore import PublishState
 from xmodule.modulestore.django import modulestore
 
 from xblock.core import XBlock
@@ -96,11 +95,7 @@ def subsection_handler(request, usage_key_string):
 
         can_view_live = False
         subsection_units = item.get_children()
-        for unit in subsection_units:
-            has_published = modulestore().compute_publish_state(unit) != PublishState.private
-            if has_published:
-                can_view_live = True
-                break
+        can_view_live = any([modulestore().has_published_version(unit) for unit in subsection_units])
 
         return render_to_response(
             'edit_subsection.html',

cms/djangoapps/contentstore/views/item.py

@@ -24,7 +24,7 @@ from xblock.fragment import Fragment
 
 import xmodule
 from xmodule.tabs import StaticTab, CourseTabList
-from xmodule.modulestore import ModuleStoreEnum, PublishState, EdxJSONEncoder
+from xmodule.modulestore import ModuleStoreEnum, EdxJSONEncoder
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
 from xmodule.modulestore.inheritance import own_metadata
@@ -387,8 +387,7 @@ def _save_xblock(user, xblock, data=None, children=None, metadata=None, nullout=
     # then this item should be republished. This is used by staff locking to ensure that changing the draft
     # value of the staff lock will also update the published version, but only at the unit level.
     if publish == 'republish' and xblock.category not in DIRECT_ONLY_CATEGORIES:
-        published = modulestore().compute_publish_state(xblock) != PublishState.private
-        if published:
+        if modulestore().has_published_version(xblock):
             publish = 'make_public'
 
     # Make public after updating the xblock, in case the caller asked for both an update and a publish.
@@ -659,7 +658,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F
         visibility_state = _compute_visibility_state(xblock, child_info, is_xblock_unit and has_changes)
     else:
         visibility_state = None
-    published = modulestore().compute_publish_state(xblock) != PublishState.private
+    published = modulestore().has_published_version(xblock)
 
     xblock_info = {
         "id": unicode(xblock.location),

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

@@ -90,16 +90,6 @@ class ModuleStoreEnum(object):
         test = -3
 
 
-class PublishState(object):
-    """
-    The legacy publish state for a given xblock-- either 'draft', 'private', or 'public'. These states
-    are no longer used in Studio, but they are still referenced in a few places in LMS.
-    """
-    draft = 'draft'
-    private = 'private'
-    public = 'public'
-
-
 class ModuleStoreRead(object):
     """
     An abstract interface for a database backend that stores XModuleDescriptor
@@ -307,15 +297,9 @@ class ModuleStoreRead(object):
         pass
 
     @abstractmethod
-    def compute_publish_state(self, xblock):
+    def has_published_version(self, xblock):
         """
-        Returns whether this xblock is draft, public, or private.
-
-        Returns:
-            PublishState.draft - content is in the process of being edited, but still has a previous
-                version deployed to LMS
-            PublishState.public - content is locked and deployed to LMS
-            PublishState.private - content is editable and not deployed to LMS
+        Returns true if this xblock exists in the published course regardless of whether it's up to date
         """
         pass
 
@@ -529,11 +513,11 @@ class ModuleStoreReadBase(ModuleStoreRead):
                 None
             )
 
-    def compute_publish_state(self, xblock):
+    def has_published_version(self, xblock):
         """
-        Returns PublishState.public since this is a read-only store.
+        Returns True since this is a read-only store.
         """
-        return PublishState.public
+        return True
 
     def heartbeat(self):
         """

common/lib/xmodule/xmodule/modulestore/draft_and_published.py

@@ -96,7 +96,7 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin):
         raise NotImplementedError
 
     @abstractmethod
-    def compute_publish_state(self, xblock):
+    def has_published_version(self, xblock):
         raise NotImplementedError
 
     @abstractmethod

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -540,7 +540,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
             )
         )
 
-    def compute_publish_state(self, xblock):
+    def has_published_version(self, xblock):
         """
         Returns whether this xblock is draft, public, or private.
 
@@ -552,7 +552,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
         """
         course_id = xblock.scope_ids.usage_id.course_key
         store = self._get_modulestore_for_courseid(course_id)
-        return store.compute_publish_state(xblock)
+        return store.has_published_version(xblock)
 
     @strip_key
     def publish(self, location, user_id, **kwargs):

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

@@ -11,7 +11,7 @@ import logging
 
 from opaque_keys.edx.locations import Location
 from xmodule.exceptions import InvalidVersionError
-from xmodule.modulestore import PublishState, ModuleStoreEnum
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError, DuplicateCourseError
 from xmodule.modulestore.mongo.base import (
     MongoModuleStore, MongoRevisionKey, as_draft, as_published,
@@ -591,13 +591,13 @@ class DraftModuleStore(MongoModuleStore):
 
     def has_changes(self, xblock):
         """
-        Check if the xblock or its children have been changed since the last publish.
+        Check if the subtree rooted at xblock has any drafts and thus may possibly have changes
         :param xblock: xblock to check
-        :return: True if the draft and published versions differ
+        :return: True if there are any drafts anywhere in the subtree under xblock (a weaker
+            condition than for other stores)
         """
-
         # don't check children if this block has changes (is not public)
-        if self.compute_publish_state(xblock) != PublishState.public:
+        if getattr(xblock, 'is_draft', False):
             return True
         # if this block doesn't have changes, then check its children
         elif xblock.has_children:
@@ -771,25 +771,18 @@ class DraftModuleStore(MongoModuleStore):
 
         return queried_children
 
-    def compute_publish_state(self, xblock):
+    def has_published_version(self, xblock):
         """
-        Returns whether this xblock is draft, public, or private.
-
-        Returns:
-            PublishState.draft - content is in the process of being edited, but still has a previous
-                version deployed to LMS
-            PublishState.public - content is locked and deployed to LMS
-            PublishState.private - content is editable and not deployed to LMS
+        Returns True if this xblock has an existing published version regardless of whether the
+        published version is up to date.
         """
         if getattr(xblock, 'is_draft', False):
             published_xblock_location = as_published(xblock.location)
             try:
                 xblock.runtime.lookup_item(published_xblock_location)
             except ItemNotFoundError:
-                return PublishState.private
-            return PublishState.draft
-        else:
-            return PublishState.public
+                return False
+        return True
 
     def _verify_branch_setting(self, expected_branch_setting):
         """

common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py

@@ -3,7 +3,7 @@ Module for the dual-branch fall-back Draft->Published Versioning ModuleStore
 """
 
 from split import SplitMongoModuleStore, EXCLUDE_ALL
-from xmodule.modulestore import ModuleStoreEnum, PublishState
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.exceptions import InsufficientSpecificationError
 from xmodule.modulestore.draft_and_published import (
     ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES, UnsupportedRevisionError
@@ -302,27 +302,11 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
         block_locator = self._map_revision_to_branch(block_locator)
         return super(DraftVersioningModuleStore, self).get_block_generations(block_locator)
 
-    def compute_publish_state(self, xblock):
+    def has_published_version(self, xblock):
         """
-        Returns whether this xblock is draft, public, or private.
-
-        Returns:
-            PublishState.draft - published exists and is different from draft
-            PublishState.public - published exists and is the same as draft
-            PublishState.private - no published version exists
+        Returns whether this xblock has a published version (whether it's up to date or not).
         """
-        draft_head = self._get_head(xblock, ModuleStoreEnum.BranchName.draft)
-        published_head = self._get_head(xblock, ModuleStoreEnum.BranchName.published)
-
-        if not published_head:
-            # published version does not exist
-            return PublishState.private
-        elif self._get_version(draft_head) == self._get_version(published_head):
-            # published and draft versions are equal
-            return PublishState.public
-        else:
-            # published and draft versions differ
-            return PublishState.draft
+        return self._get_head(xblock, ModuleStoreEnum.BranchName.published) is not None
 
     def convert_to_draft(self, location, user_id):
         """

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

@@ -9,7 +9,7 @@ import datetime
 from pytz import UTC
 
 from xmodule.tests import DATA_DIR
-from xmodule.modulestore import ModuleStoreEnum, PublishState
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.exceptions import InvalidVersionError
 
@@ -988,11 +988,11 @@ class TestMixedModuleStore(unittest.TestCase):
         )
         self.assertIsNotNone(draft_xblock)
 
-    @ddt.data(('draft', 1, 0), ('split', 4, 0))
+    @ddt.data(('draft', 1, 0), ('split', 2, 0))
     @ddt.unpack
-    def test_compute_publish_state(self, default_ms, max_find, max_send):
+    def test_has_published_version(self, default_ms, max_find, max_send):
         """
-        Test the compute_publish_state method
+        Test the has_published_version method
         """
         self.initdb(default_ms)
         self._create_block_hierarchy()
@@ -1002,37 +1002,33 @@ class TestMixedModuleStore(unittest.TestCase):
         item_location = item.location
         mongo_store = self.store._get_modulestore_for_courseid(self._course_key_from_string(self.MONGO_COURSEID))
         with check_mongo_calls(mongo_store, max_find, max_send):
-            self.assertEquals(self.store.compute_publish_state(item), PublishState.private)
+            self.assertFalse(self.store.has_published_version(item))
 
         # Private -> Public
         self.store.publish(item_location, self.user_id)
         item = self.store.get_item(item_location)
-        self.assertEquals(self.store.compute_publish_state(item), PublishState.public)
+        self.assertTrue(self.store.has_published_version(item))
 
         # Public -> Private
         self.store.unpublish(item_location, self.user_id)
         item = self.store.get_item(item_location)
-        self.assertEquals(self.store.compute_publish_state(item), PublishState.private)
+        self.assertFalse(self.store.has_published_version(item))
 
         # Private -> Public
         self.store.publish(item_location, self.user_id)
         item = self.store.get_item(item_location)
-        self.assertEquals(self.store.compute_publish_state(item), PublishState.public)
+        self.assertTrue(self.store.has_published_version(item))
 
         # Public -> Draft with NO changes
-        # Note: This is where Split and Mongo differ
         self.store.convert_to_draft(item_location, self.user_id)
         item = self.store.get_item(item_location)
-        self.assertEquals(
-            self.store.compute_publish_state(item),
-            PublishState.draft if default_ms == 'draft' else PublishState.public
-        )
+        self.assertTrue(self.store.has_published_version(item))
 
         # Draft WITH changes
         item.display_name = 'new name'
         item = self.store.update_item(item, self.user_id)
         self.assertTrue(self.store.has_changes(item))
-        self.assertEquals(self.store.compute_publish_state(item), PublishState.draft)
+        self.assertTrue(self.store.has_published_version(item))
 
     @ddt.data('draft', 'split')
     def test_auto_publish(self, default_ms):
@@ -1047,47 +1043,47 @@ class TestMixedModuleStore(unittest.TestCase):
 
         # test create_course to make sure we are autopublishing
         test_course = self.store.create_course('testx', 'GreekHero', 'test_run', self.user_id)
-        self.assertEqual(self.store.compute_publish_state(test_course), PublishState.public)
+        self.assertTrue(self.store.has_published_version(test_course))
 
         test_course_key = test_course.id
 
         # test create_item of direct-only category to make sure we are autopublishing
         chapter = self.store.create_item(self.user_id, test_course_key, 'chapter', 'Overview')
-        self.assertEqual(self.store.compute_publish_state(chapter), PublishState.public)
+        self.assertTrue(self.store.has_published_version(chapter))
 
         chapter_location = chapter.location
 
         # test create_child of direct-only category to make sure we are autopublishing
         sequential = self.store.create_child(self.user_id, chapter_location, 'sequential', 'Sequence')
-        self.assertEqual(self.store.compute_publish_state(sequential), PublishState.public)
+        self.assertTrue(self.store.has_published_version(sequential))
 
         # test update_item of direct-only category to make sure we are autopublishing
         sequential.display_name = 'sequential1'
         sequential = self.store.update_item(sequential, self.user_id)
-        self.assertEqual(self.store.compute_publish_state(sequential), PublishState.public)
+        self.assertTrue(self.store.has_published_version(sequential))
 
         # test delete_item of direct-only category to make sure we are autopublishing
         self.store.delete_item(sequential.location, self.user_id, revision=ModuleStoreEnum.RevisionOption.all)
         chapter = self.store.get_item(chapter.location.for_branch(None))
-        self.assertEqual(self.store.compute_publish_state(chapter), PublishState.public)
+        self.assertTrue(self.store.has_published_version(chapter))
 
         # test create_child of NOT direct-only category to make sure we aren't autopublishing
         problem_child = self.store.create_child(self.user_id, chapter_location, 'problem', 'Problem_Child')
-        self.assertEqual(self.store.compute_publish_state(problem_child), PublishState.private)
+        self.assertFalse(self.store.has_published_version(problem_child))
 
         # test create_item of NOT direct-only category to make sure we aren't autopublishing
         problem_item = self.store.create_item(self.user_id, test_course_key, 'problem', 'Problem_Item')
-        self.assertEqual(self.store.compute_publish_state(problem_item), PublishState.private)
+        self.assertFalse(self.store.has_published_version(problem_item))
 
         # test update_item of NOT direct-only category to make sure we aren't autopublishing
         problem_item.display_name = 'Problem_Item1'
         problem_item = self.store.update_item(problem_item, self.user_id)
-        self.assertEqual(self.store.compute_publish_state(problem_item), PublishState.private)
+        self.assertFalse(self.store.has_published_version(problem_item))
 
         # test delete_item of NOT direct-only category to make sure we aren't autopublishing
         self.store.delete_item(problem_child.location, self.user_id)
         chapter = self.store.get_item(chapter.location.for_branch(None))
-        self.assertEqual(self.store.compute_publish_state(chapter), PublishState.public)
+        self.assertTrue(self.store.has_published_version(chapter))
 
     @ddt.data('draft', 'split')
     def test_get_courses_for_wiki_shared(self, default_ms):

common/lib/xmodule/xmodule/tests/__init__.py

@@ -23,7 +23,7 @@ from xmodule.modulestore.inheritance import InheritanceMixin, own_metadata
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xmodule.mako_module import MakoDescriptorSystem
 from xmodule.error_module import ErrorDescriptor
-from xmodule.modulestore import PublishState, ModuleStoreEnum
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.mongo.draft import DraftModuleStore
 from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES