Modified topic and story models (oppia#5049)

* Merged with controllers * Merged with add-copy * Modified topic domain and services * fixed errors * Added tests and modified functions * modified stories * made review changes * Added test * made review changes * Added remove skill id * made review changes
quantumferret · Jun 7, 2018 · c18c473 · c18c473
1 parent 561c895
commit c18c473
Show file tree

Hide file tree

Showing 18 changed files with 823 additions and 139 deletions.
diff --git a/core/controllers/skill_editor_test.py b/core/controllers/skill_editor_test.py
@@ -40,7 +40,7 @@ def setUp(self):
         self.topic_id = topic_services.get_new_topic_id()
         self.save_new_topic(
             self.topic_id, self.admin_id, 'Name', 'Description',
-            [], [], [self.skill_id])
+            [], [], [self.skill_id], [])
 
 
 class SkillEditorTest(BaseSkillEditorControllerTest):

diff --git a/core/controllers/story_editor_test.py b/core/controllers/story_editor_test.py
@@ -41,7 +41,7 @@ def setUp(self):
         self.topic_id = topic_services.get_new_topic_id()
         self.save_new_topic(
             self.topic_id, self.admin_id, 'Name', 'Description',
-            [self.story_id], [], [])
+            [self.story_id], [], [], [])
 
 
 class StoryEditorTest(BaseStoryEditorControllerTest):

diff --git a/core/controllers/topic_editor_test.py b/core/controllers/topic_editor_test.py
@@ -46,7 +46,7 @@ def setUp(self):
         self.new_user = user_services.UserActionsInfo(self.new_user_id)
         self.topic_id = topic_services.get_new_topic_id()
         self.save_new_topic(
-            self.topic_id, self.admin_id, 'Name', 'Description', [], [], [])
+            self.topic_id, self.admin_id, 'Name', 'Description', [], [], [], [])
 
 
 class NewStoryHandlerTest(BaseTopicEditorControllerTest):
@@ -109,7 +109,7 @@ def test_access_topic_editor_page(self):
             topic_id_2 = topic_services.get_new_topic_id()
             self.save_new_topic(
                 topic_id_2, self.admin_id, 'Name', 'Description',
-                [], [], [])
+                [], [], [], [])
             self.signup('topicmanager2@example.com', 'topicmanager2')
             topic_manager_id_2 = self.get_user_id_from_email(
                 'topicmanager2@example.com')

diff --git a/core/controllers/topics_and_skills_dashboard.py b/core/controllers/topics_and_skills_dashboard.py
@@ -86,7 +86,8 @@ def post(self):
         skill_services.save_new_skill(self.user_id, skill)
 
         if topic_id is not None:
-            topic_services.add_skill(self.user_id, topic_id, new_skill_id)
+            topic_services.add_uncategorized_skill(
+                self.user_id, topic_id, new_skill_id)
 
         self.render_json({
             'skillId': new_skill_id

diff --git a/core/controllers/topics_and_skills_dashboard_test.py b/core/controllers/topics_and_skills_dashboard_test.py
@@ -30,7 +30,7 @@ def setUp(self):
         self.set_admins([self.ADMIN_USERNAME])
         self.topic_id = topic_services.get_new_topic_id()
         self.save_new_topic(
-            self.topic_id, self.admin_id, 'Name', 'Description', [], [], [])
+            self.topic_id, self.admin_id, 'Name', 'Description', [], [], [], [])
 
 
 class NewTopicHandlerTest(BaseTopicsAndSkillsDashboardTest):
@@ -101,5 +101,5 @@ def test_skill_creation_in_valid_topic(self):
             self.assertIsNotNone(
                 skill_services.get_skill_by_id(skill_id, strict=False))
             topic = topic_services.get_topic_by_id(self.topic_id)
-            self.assertEqual(topic.skill_ids, [skill_id])
+            self.assertEqual(topic.uncategorized_skill_ids, [skill_id])
             self.logout()
diff --git a/core/domain/skill_domain.py b/core/domain/skill_domain.py
@@ -211,7 +211,7 @@ def validate(self):
 
         Raises:
             ValidationError: One or more attributes of the misconception are
-            invalid.
+                invalid.
         """
         if not isinstance(self.name, basestring):
             raise utils.ValidationError(

diff --git a/core/domain/skill_services.py b/core/domain/skill_services.py
@@ -41,7 +41,7 @@ def _migrate_skill_contents_to_latest_schema(versioned_skill_contents):
 
     Raises:
         Exception: The schema version of the skill_contents is outside of what
-        is supported at present.
+            is supported at present.
     """
     skill_contents_schema_version = versioned_skill_contents['schema_version']
     if not (1 <= skill_contents_schema_version
@@ -72,7 +72,7 @@ def _migrate_misconceptions_to_latest_schema(versioned_misconceptions):
 
     Raises:
         Exception: The schema version of misconceptions is outside of what
-        is supported at present.
+            is supported at present.
     """
     misconception_schema_version = versioned_misconceptions['schema_version']
     if not (1 <= misconception_schema_version

diff --git a/core/domain/story_domain.py b/core/domain/story_domain.py
@@ -35,6 +35,7 @@
 STORY_NODE_PROPERTY_OUTLINE = 'outline'
 STORY_NODE_PROPERTY_EXPLORATION_ID = 'exploration_id'
 
+
 INITIAL_NODE_ID = 'initial_node_id'
 
 CMD_MIGRATE_SCHEMA_TO_LATEST_VERSION = 'migrate_schema_to_latest_version'
@@ -48,6 +49,7 @@
 # These take node_id as parameter.
 CMD_ADD_STORY_NODE = 'add_story_node'
 CMD_DELETE_STORY_NODE = 'delete_story_node'
+CMD_UPDATE_STORY_NODE_OUTLINE_STATUS = 'update_story_node_outline_status'
 
 # This takes additional 'title' parameters.
 CMD_CREATE_NEW = 'create_new'
@@ -72,7 +74,7 @@ class StoryChange(object):
 
     OPTIONAL_CMD_ATTRIBUTE_NAMES = [
         'property_name', 'new_value', 'old_value', 'node_id', 'from_version',
-        'to_version', 'title'
+        'to_version', 'title', 'finalized'
     ]
 
     def __init__(self, change_dict):
@@ -99,6 +101,10 @@ def __init__(self, change_dict):
             self.node_id = change_dict['node_id']
         elif self.cmd == CMD_DELETE_STORY_NODE:
             self.node_id = change_dict['node_id']
+        elif self.cmd == CMD_UPDATE_STORY_NODE_OUTLINE_STATUS:
+            self.node_id = change_dict['node_id']
+            self.old_value = change_dict['old_value']
+            self.new_value = change_dict['new_value']
         elif self.cmd == CMD_UPDATE_STORY_NODE_PROPERTY:
             if (change_dict['property_name'] not in
                     self.STORY_NODE_PROPERTIES):
@@ -152,7 +158,7 @@ class StoryNode(object):
     def __init__(
             self, node_id, destination_node_ids,
             acquired_skill_ids, prerequisite_skill_ids,
-            outline, exploration_id):
+            outline, outline_is_finalized, exploration_id):
         """Initializes a StoryNode domain object.
 
         Args:
@@ -167,6 +173,8 @@ def __init__(
                 can use to construct the exploration. It describes the basic
                 theme or template of the story and is to be provided in html
                 form.
+            outline_is_finalized: bool. Whether the outline for the story
+                node is finalized or not.
             exploration_id: str or None. The valid exploration id that fits the
                 story node. It can be None initially, when the story creator
                 has just created a story with the basic storyline (by providing
@@ -177,6 +185,7 @@ def __init__(
         self.acquired_skill_ids = acquired_skill_ids
         self.prerequisite_skill_ids = prerequisite_skill_ids
         self.outline = html_cleaner.clean(outline)
+        self.outline_is_finalized = outline_is_finalized
         self.exploration_id = exploration_id
 
     @classmethod
@@ -230,6 +239,7 @@ def to_dict(self):
             'acquired_skill_ids': self.acquired_skill_ids,
             'prerequisite_skill_ids': self.prerequisite_skill_ids,
             'outline': self.outline,
+            'outline_is_finalized': self.outline_is_finalized,
             'exploration_id': self.exploration_id
         }
 
@@ -247,7 +257,7 @@ def from_dict(cls, node_dict):
             node_dict['id'], node_dict['destination_node_ids'],
             node_dict['acquired_skill_ids'],
             node_dict['prerequisite_skill_ids'], node_dict['outline'],
-            node_dict['exploration_id'])
+            node_dict['outline_is_finalized'], node_dict['exploration_id'])
 
         return node
 
@@ -262,7 +272,7 @@ def create_default_story_node(cls, node_id):
             StoryNode. The StoryNode domain object with default
             value.
         """
-        return cls(node_id, [], [], [], '', None)
+        return cls(node_id, [], [], [], '', False, None)
 
     def validate(self):
         """Validates various properties of the story node.
@@ -282,6 +292,11 @@ def validate(self):
                 'Expected outline to be a string, received %s' %
                 self.outline)
 
+        if not isinstance(self.outline_is_finalized, bool):
+            raise utils.ValidationError(
+                'Expected outline_is_finalized to be a boolean, received %s' %
+                self.outline_is_finalized)
+
         self.require_valid_node_id(self.id)
 
         if not isinstance(self.prerequisite_skill_ids, list):
@@ -779,6 +794,38 @@ def update_node_outline(self, node_id, new_outline):
                 'The node with id %s is not part of this story' % node_id)
         self.story_contents.nodes[node_index].outline = new_outline
 
+    def mark_node_outline_as_finalized(self, node_id):
+        """Updates the outline_is_finalized field of the node with the given
+        node_id as True.
+
+        Args:
+            node_id: str. The id of the node.
+
+        Raises:
+            ValueError: The node is not part of the story.
+        """
+        node_index = self.story_contents.get_node_index(node_id)
+        if node_index is None:
+            raise ValueError(
+                'The node with id %s is not part of this story' % node_id)
+        self.story_contents.nodes[node_index].outline_is_finalized = True
+
+    def mark_node_outline_as_unfinalized(self, node_id):
+        """Updates the outline_is_finalized field of the node with the given
+        node_id as False.
+
+        Args:
+            node_id: str. The id of the node.
+
+        Raises:
+            ValueError: The node is not part of the story.
+        """
+        node_index = self.story_contents.get_node_index(node_id)
+        if node_index is None:
+            raise ValueError(
+                'The node with id %s is not part of this story' % node_id)
+        self.story_contents.nodes[node_index].outline_is_finalized = False
+
     def update_node_acquired_skill_ids(self, node_id, new_acquired_skill_ids):
         """Updates the acquired skill ids field of a given node.
 

diff --git a/core/domain/story_domain_test.py b/core/domain/story_domain_test.py
@@ -73,6 +73,7 @@ def test_defaults(self):
                     'acquired_skill_ids': [],
                     'prerequisite_skill_ids': [],
                     'outline': '',
+                    'outline_is_finalized': False,
                     'exploration_id': None
                 }],
                 'initial_node_id': self.NODE_ID_1,
@@ -129,6 +130,11 @@ def test_get_number_from_node_id(self):
         self.assertEqual(
             story_domain.StoryNode.get_number_from_node_id('node_10'), 10)
 
+    def test_node_outline_finalized_validation(self):
+        self.story.story_contents.nodes[0].outline_is_finalized = 'abs'
+        self._assert_validation_error(
+            'Expected outline_is_finalized to be a boolean')
+
     def test_nodes_validation(self):
         self.story.story_contents.initial_node_id = 'node_10'
         self._assert_validation_error('Expected starting node to exist')
@@ -153,6 +159,7 @@ def test_nodes_validation(self):
                 'prerequisite_skill_ids': [],
                 'acquired_skill_ids': [],
                 'outline': 'Outline',
+                'outline_is_finalized': False,
                 'exploration_id': 'exploration_id'
             })
         ]
@@ -203,6 +210,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_2'],
             'prerequisite_skill_ids': ['skill_1'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_2 = {
@@ -211,6 +219,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_3'],
             'prerequisite_skill_ids': ['skill_2'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_3 = {
@@ -219,6 +228,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_4'],
             'prerequisite_skill_ids': ['skill_3'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         self.story.story_contents.initial_node_id = 'node_1'
@@ -238,6 +248,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_2'],
             'prerequisite_skill_ids': ['skill_1'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_2 = {
@@ -246,6 +257,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_3'],
             'prerequisite_skill_ids': ['skill_2'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_3 = {
@@ -254,6 +266,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_4'],
             'prerequisite_skill_ids': ['skill_3'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         self.story.story_contents.nodes = [
@@ -270,6 +283,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_2'],
             'prerequisite_skill_ids': ['skill_1'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_2 = {
@@ -278,6 +292,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_3'],
             'prerequisite_skill_ids': ['skill_2'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_3 = {
@@ -286,6 +301,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_4'],
             'prerequisite_skill_ids': ['skill_3'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         self.story.story_contents.nodes = [
@@ -303,6 +319,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_2'],
             'prerequisite_skill_ids': ['skill_1'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_2 = {
@@ -311,6 +328,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_3'],
             'prerequisite_skill_ids': ['skill_2'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_3 = {
@@ -319,6 +337,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_4'],
             'prerequisite_skill_ids': ['skill_3'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         self.story.story_contents.nodes = [
@@ -337,6 +356,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_2'],
             'prerequisite_skill_ids': ['skill_1', 'skill_0'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_2 = {
@@ -345,6 +365,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': ['skill_3', 'skill_4'],
             'prerequisite_skill_ids': ['skill_2'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_3 = {
@@ -353,6 +374,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': [],
             'prerequisite_skill_ids': ['skill_4'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         node_4 = {
@@ -361,6 +383,7 @@ def test_all_nodes_visited(self):
             'acquired_skill_ids': [],
             'prerequisite_skill_ids': ['skill_2'],
             'outline': '',
+            'outline_is_finalized': False,
             'exploration_id': None
         }
         self.story.story_contents.nodes = [
@@ -378,7 +401,7 @@ def test_story_contents_export_import(self):
         story_node = story_domain.StoryNode(
             self.NODE_ID_1, [self.NODE_ID_2],
             [self.SKILL_ID_1], [self.SKILL_ID_2],
-            'Outline', self.EXP_ID)
+            'Outline', False, self.EXP_ID)
         story_contents = story_domain.StoryContents(
             [story_node], self.NODE_ID_1, 2)
         story_contents_dict = story_contents.to_dict()