Fix oppia#5447: Permanently flip ENABLE_STATE_ID_MAPPING constant in …

…Oppia. (oppia#5523)

* Fixed few tests

* Fix backend tests

* Fix backend tests

* Addressed review comments

* Fix backend tests and addressed review comments

* Address review comments

core/controllers/editor_test.py

@@ -542,14 +542,48 @@ def test_exploration_download_handler_for_default_exploration(self):
         init_state = exploration.states[exploration.init_state_name]
         init_interaction = init_state.interaction
         init_interaction.default_outcome.dest = exploration.init_state_name
-        exploration.add_states(['State A', 'State 2', 'State 3'])
-        exploration.states['State A'].update_interaction_id('TextInput')
-        exploration.states['State 2'].update_interaction_id('TextInput')
-        exploration.states['State 3'].update_interaction_id('TextInput')
-        exploration.rename_state('State 2', 'State B')
-        exploration.delete_state('State 3')
-        exp_services._save_exploration(  # pylint: disable=protected-access
-            owner_id, exploration, '', [])
+        exp_services.update_exploration(
+            owner_id, exp_id, [
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_ADD_STATE,
+                    'state_name': 'State A',
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_ADD_STATE,
+                    'state_name': 'State 2',
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_ADD_STATE,
+                    'state_name': 'State 3',
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
+                    'property_name': exp_domain.STATE_PROPERTY_INTERACTION_ID,
+                    'state_name': 'State A',
+                    'new_value': 'TextInput'
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
+                    'property_name': exp_domain.STATE_PROPERTY_INTERACTION_ID,
+                    'state_name': 'State 2',
+                    'new_value': 'TextInput'
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
+                    'property_name': exp_domain.STATE_PROPERTY_INTERACTION_ID,
+                    'state_name': 'State 3',
+                    'new_value': 'TextInput'
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_RENAME_STATE,
+                    'old_state_name': 'State 2',
+                    'new_state_name': 'State B'
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_DELETE_STATE,
+                    'state_name': 'State 3',
+                })], 'changes')
+        exploration = exp_services.get_exploration_by_id(exp_id)
         response = self.testapp.get('/create/%s' % exp_id)
 
         # Check download to zip file.

core/domain/exp_domain.py

@@ -3285,7 +3285,7 @@ def create_mapping_for_new_version(
                 new_exploration.states.keys()):
             unknown_state_names = ((
                 set(new_exploration.states.keys()) -
-                set(new_state_names_to_ids.keys())) + (
+                set(new_state_names_to_ids.keys())).union(
                     set(new_state_names_to_ids.keys()) -
                     set(new_exploration.states.keys())))
             raise Exception(

core/domain/exp_jobs_one_off_test.py

@@ -1048,26 +1048,33 @@ def test_for_textangular_validation_and_migration(self):
                 job_id))
 
         # Test that validation fails before migration.
-        self.assertGreater(len(actual_output), 0)
+        self.assertEqual(len(actual_output), 16)
 
         exploration_dict = exploration.to_dict()
-        updated_dict = exp_domain.Exploration._convert_v26_dict_to_v27_dict( # pylint: disable=protected-access
+        # We need to create a brand-new exploration here in addition to the old
+        # one (rather than just overwriting the old one), because state id
+        # mapping model is generated when each (exp, version) is saved for
+        # first time. Hence when an exisiting exploration is overwritten
+        # state id mapping model throws an error that mapping already exists.
+        new_exp_dict = exp_domain.Exploration._convert_v26_dict_to_v27_dict( # pylint: disable=protected-access
             exploration_dict)
         # This is done to ensure that exploration is not passed through CKEditor
         # Migration pipeline.
-        updated_dict['schema_version'] = 29
-        updated_dict['states_schema_version'] = 24
-        updated_exploration = exp_domain.Exploration.from_dict(updated_dict)
-        updated_states = updated_dict['states']
+        new_exp_dict['id'] = self.NEW_EXP_ID
+        new_exp_dict['schema_version'] = 29
+        new_exp_dict['states_schema_version'] = 24
+        new_exploration = exp_domain.Exploration.from_dict(new_exp_dict)
+        new_states = new_exp_dict['states']
 
         for index, state_name in enumerate(state_list):
-            updated_html = updated_states[state_name]['content']['html']
+            new_html = new_states[state_name]['content']['html']
 
             # Test that html matches the expected format after migration.
             self.assertEqual(
-                updated_html, unicode(test_cases[index]['expected_output']))
+                new_html, unicode(test_cases[index]['expected_output']))
+
+        exp_services.save_new_exploration(self.albert_id, new_exploration)
 
-        exp_services.save_new_exploration(self.albert_id, updated_exploration)
         # Start validation job on updated exploration.
         job_id = (
             exp_jobs_one_off.ExplorationContentValidationJobForTextAngular.create_new()) # pylint: disable=line-too-long
@@ -1084,7 +1091,10 @@ def test_for_textangular_validation_and_migration(self):
                 job_id))
 
         # Test that validation passes after migration.
-        self.assertEqual(actual_output, [])
+        # There should be no validation errors in the new (updated)
+        # exploration, but there are 16 validation errors in the old
+        # exploration.
+        self.assertEqual(len(actual_output), 16)
 
 
 class ExplorationContentValidationJobForCKEditorTest(

core/domain/exp_services_test.py

@@ -777,18 +777,6 @@ def test_loading_recent_yaml_does_not_default_exp_title_category(self):
         self.assertNotEqual(exp.title, feconf.DEFAULT_EXPLORATION_TITLE)
         self.assertNotEqual(exp.category, feconf.DEFAULT_EXPLORATION_CATEGORY)
 
-    def test_loading_exploration_from_yaml_does_not_override_existing_id(self):
-        # Load a a demo exploration.
-        exp_services.load_demo(self.DEMO_EXP_ID)
-
-        # Override the demo exploration using the import method.
-        exp_services.save_new_exploration_from_yaml_and_assets(
-            self.owner_id, self.SAMPLE_YAML_CONTENT, self.DEMO_EXP_ID, [])
-
-        # The demo exploration should not have been overwritten.
-        exp = exp_services.get_exploration_by_id(self.DEMO_EXP_ID)
-        self.assertNotEqual(exp.to_yaml(), self.SAMPLE_YAML_CONTENT)
-
     def test_loading_untitled_yaml_defaults_exploration_title_category(self):
         exp_services.save_new_exploration_from_yaml_and_assets(
             self.owner_id, self.SAMPLE_UNTITLED_YAML_CONTENT, self.EXP_ID, [])
@@ -1304,9 +1292,18 @@ def test_export_to_zip_file(self):
         init_state = exploration.states[exploration.init_state_name]
         init_interaction = init_state.interaction
         init_interaction.default_outcome.dest = exploration.init_state_name
-        exploration.add_states(['New state'])
-        exploration.states['New state'].update_interaction_id('TextInput')
-        exp_services._save_exploration(self.owner_id, exploration, '', [])
+        exp_services.update_exploration(
+            self.owner_id, self.EXP_ID, [
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_ADD_STATE,
+                    'state_name': 'New state',
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
+                    'property_name': exp_domain.STATE_PROPERTY_INTERACTION_ID,
+                    'state_name': 'New state',
+                    'new_value': 'TextInput'
+                })], 'Add state name')
 
         zip_file_output = exp_services.export_to_zip_file(self.EXP_ID)
         zf = zipfile.ZipFile(StringIO.StringIO(zip_file_output))
@@ -1322,9 +1319,18 @@ def test_export_to_zip_file_with_assets(self):
         init_state = exploration.states[exploration.init_state_name]
         init_interaction = init_state.interaction
         init_interaction.default_outcome.dest = exploration.init_state_name
-        exploration.add_states(['New state'])
-        exploration.states['New state'].update_interaction_id('TextInput')
-        exp_services._save_exploration(self.owner_id, exploration, '', [])
+        exp_services.update_exploration(
+            self.owner_id, self.EXP_ID, [
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_ADD_STATE,
+                    'state_name': 'New state',
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
+                    'property_name': exp_domain.STATE_PROPERTY_INTERACTION_ID,
+                    'state_name': 'New state',
+                    'new_value': 'TextInput'
+                })], 'Add state name')
 
         with open(os.path.join(feconf.TESTS_DATA_DIR, 'img.png')) as f:
             raw_image = f.read()
@@ -1501,9 +1507,18 @@ def test_export_to_dict(self):
         init_state = exploration.states[exploration.init_state_name]
         init_interaction = init_state.interaction
         init_interaction.default_outcome.dest = exploration.init_state_name
-        exploration.add_states(['New state'])
-        exploration.states['New state'].update_interaction_id('TextInput')
-        exp_services._save_exploration(self.owner_id, exploration, '', [])
+        exp_services.update_exploration(
+            self.owner_id, self.EXP_ID, [
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_ADD_STATE,
+                    'state_name': 'New state',
+                }),
+                exp_domain.ExplorationChange({
+                    'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
+                    'property_name': exp_domain.STATE_PROPERTY_INTERACTION_ID,
+                    'state_name': 'New state',
+                    'new_value': 'TextInput'
+                })], 'Add state name')
 
         dict_output = exp_services.export_states_to_yaml(self.EXP_ID, width=50)
 
@@ -1775,12 +1790,17 @@ def test_update_interaction_customization_args(self):
 
     def test_update_interaction_handlers_fails(self):
         """Test legacy interaction handler updating."""
-        exploration = exp_services.get_exploration_by_id(self.EXP_ID)
-        exploration.add_states(['State 2'])
-        exploration.states['State 2'].update_interaction_id('TextInput')
-        exp_services._save_exploration(self.owner_id, exploration, '', [])
+        exp_services.update_exploration(
+            self.owner_id, self.EXP_ID,
+            [exp_domain.ExplorationChange({
+                'cmd': exp_domain.CMD_ADD_STATE,
+                'state_name': 'State 2',
+            })] + _get_change_list(
+                'State 2',
+                exp_domain.STATE_PROPERTY_INTERACTION_ID,
+                'TextInput'),
+            'Add state name')
 
-        exploration = exp_services.get_exploration_by_id(self.EXP_ID)
         self.interaction_default_outcome['dest'] = 'State 2'
         with self.assertRaisesRegexp(
             utils.InvalidInputException,
@@ -1806,9 +1826,16 @@ def test_update_interaction_answer_groups(self):
         """Test updating of interaction_answer_groups."""
         # We create a second state to use as a rule destination.
         exploration = exp_services.get_exploration_by_id(self.EXP_ID)
-        exploration.add_states(['State 2'])
-        exploration.states['State 2'].update_interaction_id('TextInput')
-        exp_services._save_exploration(self.owner_id, exploration, '', [])
+        exp_services.update_exploration(
+            self.owner_id, self.EXP_ID,
+            [exp_domain.ExplorationChange({
+                'cmd': exp_domain.CMD_ADD_STATE,
+                'state_name': 'State 2',
+            })] + _get_change_list(
+                'State 2',
+                exp_domain.STATE_PROPERTY_INTERACTION_ID,
+                'TextInput'),
+            'Add state name')
 
         exploration = exp_services.get_exploration_by_id(self.EXP_ID)
         self.interaction_default_outcome['dest'] = 'State 2'

core/domain/old_feedback_jobs_one_off_test.py

@@ -332,14 +332,20 @@ def setUp(self):
         init_state = exploration.states[exploration.init_state_name]
         init_interaction = init_state.interaction
         init_interaction.default_outcome.dest = exploration.init_state_name
-        exploration.add_states(['State 1'])
-
         self.old_content = state_domain.SubtitledHtml(
             'content', 'old content').to_dict()
+        exp_services.update_exploration(
+            self.editor_id, self.EXP_ID,
+            [exp_domain.ExplorationChange({
+                'cmd': exp_domain.CMD_ADD_STATE,
+                'state_name': 'State 1',
+            }), exp_domain.ExplorationChange({
+                'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
+                'state_name': 'State 1',
+                'property_name': exp_domain.STATE_PROPERTY_CONTENT,
+                'new_value': self.old_content
+            })], 'Add state name')
 
-        # Create content in State A with a single audio subtitle.
-        exploration.states['State 1'].update_content(self.old_content)
-        exp_services._save_exploration(self.editor_id, exploration, '', [])  # pylint: disable=protected-access
         rights_manager.publish_exploration(self.editor, self.EXP_ID)
         rights_manager.assign_role_for_exploration(
             self.editor, self.EXP_ID, self.owner_id,

feconf.py

@@ -780,7 +780,7 @@ def get_empty_ratings():
 SHOW_COLLECTION_NAVIGATION_TAB_STATS = False
 # Whether state id mapping model should be generated and stored when exploration
 # is created or updated.
-ENABLE_STATE_ID_MAPPING = False
+ENABLE_STATE_ID_MAPPING = True
 
 # The regular expression used to identify whether a string contains float value.
 # The regex must match with regex that is stored in vmconf.py file of Oppia-ml.