diff --git a/core/domain/draft_upgrade_services.py b/core/domain/draft_upgrade_services.py index 0c00b742026a..49a9144074b3 100644 --- a/core/domain/draft_upgrade_services.py +++ b/core/domain/draft_upgrade_services.py @@ -82,6 +82,37 @@ def try_upgrading_draft_to_exp_version( class DraftUpgradeUtil(object): """Wrapper class that contains util functions to upgrade drafts.""" + @classmethod + def _convert_states_v29_dict_to_v30_dict(cls, draft_change_list): + """Converts draft change list from state version 29 to 30. State + version 30 replaces tagged_misconception_id with + tagged_skill_misconception_id. + + Args: + draft_change_list: list(ExplorationChange). The list of + ExplorationChange domain objects to upgrade. + + Returns: + list(ExplorationChange). The converted draft_change_list. + """ + for i, change in enumerate(draft_change_list): + if (change.cmd == exp_domain.CMD_EDIT_STATE_PROPERTY and + change.property_name == + exp_domain.STATE_PROPERTY_INTERACTION_ANSWER_GROUPS): + draft_change_list[i] = exp_domain.ExplorationChange({ + 'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY, + 'property_name': ( + exp_domain.STATE_PROPERTY_INTERACTION_ANSWER_GROUPS), + 'state_name': change.state_name, + 'new_value': { + 'rule_specs': change.new_value['rule_specs'], + 'outcome': change.new_value['outcome'], + 'training_data': change.new_value['training_data'], + 'tagged_skill_misconception_id': None + } + }) + return draft_change_list + @classmethod def _convert_states_v28_dict_to_v29_dict(cls, draft_change_list): """Converts draft change list from state version 28 to 29. State diff --git a/core/domain/draft_upgrade_services_test.py b/core/domain/draft_upgrade_services_test.py index 9f8b54648f04..fa0e0870e0fe 100644 --- a/core/domain/draft_upgrade_services_test.py +++ b/core/domain/draft_upgrade_services_test.py @@ -117,6 +117,74 @@ def test_convert_to_latest_schema_version_implemented(self): msg='Current schema version is %d but DraftUpgradeUtil.%s is ' 'unimplemented.' % (state_schema_version, conversion_fn_name)) + def test_convert_states_v29_dict_to_v30_dict(self): + draft_change_list = [ + exp_domain.ExplorationChange({ + 'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY, + 'property_name': 'answer_groups', + 'state_name': 'State 1', + 'new_value': { + 'rule_specs': [{ + 'rule_type': 'Equals', + 'inputs': {'x': [ + '

This is value1 for ItemSelection

' + ]} + }, { + 'rule_type': 'Equals', + 'inputs': {'x': [ + '

This is value2 for ItemSelection

' + ]} + }], + 'outcome': { + 'dest': 'Introduction', + 'feedback': { + 'content_id': 'feedback', + 'html': '

Outcome for state1

' + }, + 'param_changes': [], + 'labelled_as_correct': False, + 'refresher_exploration_id': None, + 'missing_prerequisite_skill_id': None + }, + 'training_data': [], + 'tagged_misconception_id': None + } + })] + self.assertEqual( + draft_upgrade_services.DraftUpgradeUtil._convert_states_v29_dict_to_v30_dict( # pylint: disable=protected-access,line-too-long + draft_change_list)[0].to_dict(), + exp_domain.ExplorationChange({ + 'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY, + 'property_name': 'answer_groups', + 'state_name': 'State 1', + 'new_value': { + 'rule_specs': [{ + 'rule_type': 'Equals', + 'inputs': {'x': [ + '

This is value1 for ItemSelection

' + ]} + }, { + 'rule_type': 'Equals', + 'inputs': {'x': [ + '

This is value2 for ItemSelection

' + ]} + }], + 'outcome': { + 'dest': 'Introduction', + 'feedback': { + 'content_id': 'feedback', + 'html': '

Outcome for state1

' + }, + 'param_changes': [], + 'labelled_as_correct': False, + 'refresher_exploration_id': None, + 'missing_prerequisite_skill_id': None + }, + 'training_data': [], + 'tagged_skill_misconception_id': None + } + }).to_dict()) + def test_convert_states_v28_dict_to_v29_dict(self): draft_change_list = [ exp_domain.ExplorationChange({ diff --git a/core/domain/exp_domain.py b/core/domain/exp_domain.py index c0624a6eb828..050873d1133d 100644 --- a/core/domain/exp_domain.py +++ b/core/domain/exp_domain.py @@ -2176,7 +2176,7 @@ def _convert_states_v28_dict_to_v29_dict(cls, states_dict): Args: states_dict: dict. A dict where each key-value pair represents, - respectively, a state name and a dict used to initalize a + respectively, a state name and a dict used to initialize a State domain object. Returns: @@ -2186,6 +2186,28 @@ def _convert_states_v28_dict_to_v29_dict(cls, states_dict): state_dict['solicit_answer_details'] = False return states_dict + @classmethod + def _convert_states_v29_dict_to_v30_dict(cls, states_dict): + """Converts from version 29 to 30. Version 30 replaces + tagged_misconception_id with tagged_skill_misconception_id, which + contains the skill id and misconception id of the tagged misconception, + connected by '-'. + + Args: + states_dict: dict. A dict where each key-value pair represents, + respectively, a state name and a dict used to initialize a + State domain object. + + Returns: + dict. The converted states_dict. + """ + for state_dict in states_dict.itervalues(): + answer_groups = state_dict['interaction']['answer_groups'] + for answer_group in answer_groups: + answer_group['tagged_skill_misconception_id'] = None + del answer_group['tagged_misconception_id'] + return states_dict + @classmethod def update_states_from_model( cls, versioned_exploration_states, current_states_schema_version, @@ -2221,7 +2243,7 @@ def update_states_from_model( # incompatible changes are made to the exploration schema in the YAML # definitions, this version number must be changed and a migration process # put in place. - CURRENT_EXP_SCHEMA_VERSION = 34 + CURRENT_EXP_SCHEMA_VERSION = 35 LAST_UNTITLED_SCHEMA_VERSION = 9 @classmethod @@ -2835,6 +2857,29 @@ def _convert_v33_dict_to_v34_dict(cls, exploration_dict): return exploration_dict + @classmethod + def _convert_v34_dict_to_v35_dict(cls, exploration_dict): + """Converts a v34 exploration dict into a v35 exploration dict. + Replaces tagged_misconception_id with tagged_skill_misconception_id, + which contains the skill id and misconception id of the tagged + misconception, connected by '-'. + + Args: + exploration_dict: dict. The dict representation of an exploration + with schema version v34. + + Returns: + dict. The dict representation of the Exploration domain object, + following schema version v35. + """ + exploration_dict['schema_version'] = 35 + + exploration_dict['states'] = cls._convert_states_v29_dict_to_v30_dict( + exploration_dict['states']) + exploration_dict['states_schema_version'] = 30 + + return exploration_dict + @classmethod def _migrate_to_latest_yaml_version( cls, yaml_content, exp_id, title=None, category=None): @@ -3038,6 +3083,11 @@ def _migrate_to_latest_yaml_version( exploration_dict) exploration_schema_version = 34 + if exploration_schema_version == 34: + exploration_dict = cls._convert_v34_dict_to_v35_dict( + exploration_dict) + exploration_schema_version = 35 + return (exploration_dict, initial_schema_version) @classmethod diff --git a/core/domain/exp_domain_test.py b/core/domain/exp_domain_test.py index 8fc15430eaa0..c5ae07196b24 100644 --- a/core/domain/exp_domain_test.py +++ b/core/domain/exp_domain_test.py @@ -521,7 +521,7 @@ def test_validation(self): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }) init_state.update_interaction_answer_groups(old_answer_groups) @@ -724,7 +724,7 @@ def test_validation(self): exploration, re.escape('Hint(s) must be specified if solution is specified')) - interaction.solution = None + init_state.update_interaction_solution(None) interaction.hints = {} self._assert_validation_error( exploration, 'Expected hints to be a list') @@ -750,20 +750,49 @@ def test_validation(self): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': 'invalid_tagged_misconception_id' + 'tagged_skill_misconception_id': 1 } init_state.update_interaction_answer_groups([answer_groups_dict]) self._assert_validation_error( exploration, - 'Expected tagged misconception id to be an int, received ' - 'invalid_tagged_misconception_id') + 'Expected tagged skill misconception id to be a str, received 1') + + answer_groups_dict = { + 'outcome': { + 'dest': exploration.init_state_name, + 'feedback': { + 'content_id': 'feedback_1', + 'html': 'Feedback' + }, + 'labelled_as_correct': False, + 'param_changes': [], + 'refresher_exploration_id': None, + 'missing_prerequisite_skill_id': None + }, + 'rule_specs': [{ + 'inputs': { + 'x': 'Test' + }, + 'rule_type': 'Contains' + }], + 'training_data': [], + 'tagged_skill_misconception_id': + 'invalid_tagged_skill_misconception_id' + } + init_state.update_interaction_answer_groups([answer_groups_dict]) + + self._assert_validation_error( + exploration, + 'Expected the format of tagged skill misconception id ' + 'to be -, received ' + 'invalid_tagged_skill_misconception_id') init_state.interaction.answer_groups[0].rule_specs = {} self._assert_validation_error( exploration, 'Expected answer group rules to be a list') - init_state.interaction.answer_groups[0].tagged_misconception_id = None + init_state.interaction.answer_groups[0].tagged_skill_misconception_id = None # pylint: disable=line-too-long init_state.interaction.answer_groups[0].rule_specs = [] self._assert_validation_error( exploration, @@ -1298,7 +1327,7 @@ def test_validate_exploration_answer_group_parameter(self): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] exploration.init_state.update_interaction_answer_groups(answer_groups) @@ -4786,7 +4815,134 @@ class SchemaMigrationUnitTests(test_utils.GenericTestBase): title: Title """) - _LATEST_YAML_CONTENT = YAML_CONTENT_V34 + YAML_CONTENT_V35 = ("""author_notes: '' +auto_tts_enabled: true +blurb: '' +category: Category +correctness_feedback_enabled: false +init_state_name: (untitled state) +language_code: en +objective: '' +param_changes: [] +param_specs: {} +schema_version: 35 +states: + (untitled state): + classifier_model_id: null + content: + content_id: content + html: '' + interaction: + answer_groups: + - outcome: + dest: END + feedback: + content_id: feedback_1 + html:

Correct!

+ labelled_as_correct: false + missing_prerequisite_skill_id: null + param_changes: [] + refresher_exploration_id: null + rule_specs: + - inputs: + x: InputString + rule_type: Equals + tagged_skill_misconception_id: null + training_data: [] + confirmed_unclassified_answers: [] + customization_args: + placeholder: + value: '' + rows: + value: 1 + default_outcome: + dest: (untitled state) + feedback: + content_id: default_outcome + html: '' + labelled_as_correct: false + missing_prerequisite_skill_id: null + param_changes: [] + refresher_exploration_id: null + hints: [] + id: TextInput + solution: null + param_changes: [] + recorded_voiceovers: + voiceovers_mapping: + content: {} + default_outcome: {} + feedback_1: {} + solicit_answer_details: false + written_translations: + translations_mapping: + content: {} + default_outcome: {} + feedback_1: {} + END: + classifier_model_id: null + content: + content_id: content + html:

Congratulations, you have finished!

+ interaction: + answer_groups: [] + confirmed_unclassified_answers: [] + customization_args: + recommendedExplorationIds: + value: [] + default_outcome: null + hints: [] + id: EndExploration + solution: null + param_changes: [] + recorded_voiceovers: + voiceovers_mapping: + content: {} + solicit_answer_details: false + written_translations: + translations_mapping: + content: {} + New state: + classifier_model_id: null + content: + content_id: content + html: '' + interaction: + answer_groups: [] + confirmed_unclassified_answers: [] + customization_args: + placeholder: + value: '' + rows: + value: 1 + default_outcome: + dest: END + feedback: + content_id: default_outcome + html: '' + labelled_as_correct: false + missing_prerequisite_skill_id: null + param_changes: [] + refresher_exploration_id: null + hints: [] + id: TextInput + solution: null + param_changes: [] + recorded_voiceovers: + voiceovers_mapping: + content: {} + default_outcome: {} + solicit_answer_details: false + written_translations: + translations_mapping: + content: {} + default_outcome: {} +states_schema_version: 30 +tags: [] +title: Title +""") + + _LATEST_YAML_CONTENT = YAML_CONTENT_V35 def test_load_from_v1(self): """Test direct loading from a v1 yaml file.""" @@ -5221,7 +5377,7 @@ def test_load_from_v9(self): objective: '' param_changes: [] param_specs: {} -schema_version: 34 +schema_version: 35 states: (untitled state): classifier_model_id: null @@ -5243,7 +5399,7 @@ def test_load_from_v9(self): - inputs: x: InputString rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] confirmed_unclassified_answers: [] customization_args: @@ -5339,7 +5495,7 @@ def test_load_from_v9(self): translations_mapping: content: {} default_outcome: {} -states_schema_version: 29 +states_schema_version: 30 tags: [] title: Title """) @@ -5371,7 +5527,7 @@ def test_load_from_v12(self): objective: '' param_changes: [] param_specs: {} -schema_version: 34 +schema_version: 35 states: (untitled state): classifier_model_id: null @@ -5393,7 +5549,7 @@ def test_load_from_v12(self): - inputs: x: InputString rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] confirmed_unclassified_answers: [] customization_args: @@ -5488,7 +5644,7 @@ def test_load_from_v12(self): content: {} default_outcome: {} hint_1: {} -states_schema_version: 29 +states_schema_version: 30 tags: [] title: Title """) @@ -5538,7 +5694,7 @@ def test_load_from_v18(self): objective: '' param_changes: [] param_specs: {} -schema_version: 34 +schema_version: 35 states: (untitled state): classifier_model_id: null @@ -5560,7 +5716,7 @@ def test_load_from_v18(self): - inputs: x: InputString rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] confirmed_unclassified_answers: [] customization_args: @@ -5662,7 +5818,7 @@ def test_load_from_v18(self): default_outcome: {} hint_1: {} solution: {} -states_schema_version: 29 +states_schema_version: 30 tags: [] title: Title """) @@ -5694,7 +5850,7 @@ def test_load_from_v21(self): objective: '' param_changes: [] param_specs: {} -schema_version: 34 +schema_version: 35 states: (untitled state): classifier_model_id: null @@ -5716,7 +5872,7 @@ def test_load_from_v21(self): - inputs: x: InputString rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] confirmed_unclassified_answers: [] customization_args: @@ -5814,7 +5970,7 @@ def test_load_from_v21(self): translations_mapping: content: {} default_outcome: {} -states_schema_version: 29 +states_schema_version: 30 tags: [] title: Title """) @@ -5876,7 +6032,7 @@ def test_load_from_v29(self): objective: '' param_changes: [] param_specs: {} -schema_version: 34 +schema_version: 35 states: (untitled state): classifier_model_id: null @@ -5898,7 +6054,7 @@ def test_load_from_v29(self): - inputs: x: InputString rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] confirmed_unclassified_answers: [] customization_args: @@ -5993,7 +6149,7 @@ def test_load_from_v29(self): translations_mapping: content: {} default_outcome: {} -states_schema_version: 29 +states_schema_version: 30 tags: [] title: Title """) @@ -6447,7 +6603,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): """) # pylint: disable=line-too-long - YAML_CONTENT_V34_IMAGE_DIMENSIONS = ("""author_notes: '' + YAML_CONTENT_V35_IMAGE_DIMENSIONS = ("""author_notes: '' auto_tts_enabled: true blurb: '' category: category @@ -6457,7 +6613,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): objective: '' param_changes: [] param_specs: {} -schema_version: 34 +schema_version: 35 states: Introduction: classifier_model_id: null @@ -6555,7 +6711,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): - inputs: x: 1 rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] - outcome: dest: state3 @@ -6570,7 +6726,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): - inputs: x: 0 rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] confirmed_unclassified_answers: [] customization_args: @@ -6646,7 +6802,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): x: -

This is value3 for ItemSelectionInput

rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] confirmed_unclassified_answers: [] customization_args: @@ -6683,7 +6839,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): content: {} default_outcome: {} feedback_1: {} -states_schema_version: 29 +states_schema_version: 30 tags: [] title: title """) @@ -6798,7 +6954,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): title: Title """) - YAML_CONTENT_V34_WITH_IMAGE_CAPTION = ("""author_notes: '' + YAML_CONTENT_V35_WITH_IMAGE_CAPTION = ("""author_notes: '' auto_tts_enabled: true blurb: '' category: Category @@ -6808,7 +6964,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): objective: '' param_changes: [] param_specs: {} -schema_version: 34 +schema_version: 35 states: (untitled state): classifier_model_id: null @@ -6832,7 +6988,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): - inputs: x: InputString rule_type: Equals - tagged_misconception_id: null + tagged_skill_misconception_id: null training_data: [] confirmed_unclassified_answers: [] customization_args: @@ -6922,7 +7078,7 @@ class HTMLMigrationUnitTests(test_utils.GenericTestBase): translations_mapping: content: {} default_outcome: {} -states_schema_version: 29 +states_schema_version: 30 tags: [] title: Title """) @@ -6940,7 +7096,7 @@ def test_load_from_v26_textangular(self): exploration = exp_domain.Exploration.from_yaml( 'eid', self.YAML_CONTENT_V26_TEXTANGULAR) self.assertEqual( - exploration.to_yaml(), self.YAML_CONTENT_V34_IMAGE_DIMENSIONS) + exploration.to_yaml(), self.YAML_CONTENT_V35_IMAGE_DIMENSIONS) def test_load_from_v27_without_image_caption(self): @@ -6953,7 +7109,7 @@ def test_load_from_v27_without_image_caption(self): exploration = exp_domain.Exploration.from_yaml( 'eid', self.YAML_CONTENT_V27_WITHOUT_IMAGE_CAPTION) self.assertEqual( - exploration.to_yaml(), self.YAML_CONTENT_V34_WITH_IMAGE_CAPTION) + exploration.to_yaml(), self.YAML_CONTENT_V35_WITH_IMAGE_CAPTION) class ConversionUnitTests(test_utils.GenericTestBase): @@ -7173,7 +7329,7 @@ def test_all_html_strings_are_collected(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }, { 'rule_specs': [{ 'rule_type': 'Equals', @@ -7191,7 +7347,7 @@ def test_all_html_strings_are_collected(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] answer_group_list3 = [{ 'rule_specs': [{ @@ -7217,7 +7373,7 @@ def test_all_html_strings_are_collected(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] state2.update_interaction_answer_groups(answer_group_list2) state3.update_interaction_answer_groups(answer_group_list3) diff --git a/core/domain/exp_jobs_one_off_test.py b/core/domain/exp_jobs_one_off_test.py index 635f27089f38..d523eb178715 100644 --- a/core/domain/exp_jobs_one_off_test.py +++ b/core/domain/exp_jobs_one_off_test.py @@ -1262,7 +1262,7 @@ def test_exp_state_pairs_are_produced_only_for_desired_interactions(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] state1.update_interaction_customization_args(customization_args_dict1) @@ -1310,7 +1310,7 @@ def test_exp_state_pairs_are_produced_only_for_desired_interactions(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] state2.update_interaction_customization_args(customization_args_dict2) @@ -1375,7 +1375,7 @@ def test_no_action_is_performed_for_deleted_exploration(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] state1.update_interaction_customization_args(customization_args_dict) diff --git a/core/domain/exp_services_test.py b/core/domain/exp_services_test.py index 158a0b67241d..3c570048e467 100644 --- a/core/domain/exp_services_test.py +++ b/core/domain/exp_services_test.py @@ -130,7 +130,7 @@ def test_reverting_an_exploration_maintains_classifier_models(self): 'missing_prerequisite_skill_id': None }, 'training_data': ['answer1', 'answer2', 'answer3'], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] change_list = [exp_domain.ExplorationChange({ @@ -1311,7 +1311,7 @@ def test_get_image_filenames_from_exploration(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }, { 'rule_specs': [{ 'rule_type': 'Equals', @@ -1329,7 +1329,7 @@ def test_get_image_filenames_from_exploration(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] answer_group_list3 = [{ 'rule_specs': [{ @@ -1367,7 +1367,7 @@ def test_get_image_filenames_from_exploration(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] state2.update_interaction_answer_groups(answer_group_list2) state3.update_interaction_answer_groups(answer_group_list3) @@ -2083,7 +2083,7 @@ def setUp(self): 'missing_prerequisite_skill_id': None }, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] # Default outcome specification for an interaction. self.interaction_default_outcome = { diff --git a/core/domain/question_domain.py b/core/domain/question_domain.py index c88ff3c542e1..5906c2139272 100644 --- a/core/domain/question_domain.py +++ b/core/domain/question_domain.py @@ -203,6 +203,27 @@ def _convert_state_v28_dict_to_v29_dict(cls, question_state_dict): question_state_dict['solicit_answer_details'] = False return question_state_dict + @classmethod + def _convert_state_v29_dict_to_v30_dict(cls, question_state_dict): + """Converts from version 29 to 30. Version 30 replaces + tagged_misconception_id with tagged_skill_misconception_id, which + is default to None. + + Args: + question_state_dict: dict. A dict where each key-value pair + represents respectively, a state name and a dict used to + initalize a State domain object. + + Returns: + dict. The converted question_state_dict. + """ + answer_groups = question_state_dict['interaction']['answer_groups'] + for answer_group in answer_groups: + answer_group['tagged_skill_misconception_id'] = None + del answer_group['tagged_misconception_id'] + + return question_state_dict + @classmethod def update_state_from_model( cls, versioned_question_state, current_state_schema_version): @@ -226,6 +247,7 @@ def update_state_from_model( conversion_fn = getattr(cls, '_convert_state_v%s_dict_to_v%s_dict' % ( current_state_schema_version, current_state_schema_version + 1)) + versioned_question_state['state'] = conversion_fn( versioned_question_state['state']) diff --git a/core/domain/question_domain_test.py b/core/domain/question_domain_test.py index 66e835cbb95b..27c4226c7ee5 100644 --- a/core/domain/question_domain_test.py +++ b/core/domain/question_domain_test.py @@ -328,7 +328,7 @@ def test_strict_validation_for_answer_groups(self): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }) ] diff --git a/core/domain/question_jobs_one_off_test.py b/core/domain/question_jobs_one_off_test.py index e7a58fb4feb9..0a54f4c4f45e 100644 --- a/core/domain/question_jobs_one_off_test.py +++ b/core/domain/question_jobs_one_off_test.py @@ -120,7 +120,7 @@ def test_migration_job_converts_old_question(self): self.QUESTION_ID, self.albert_id, [self.skill_id]) question = ( question_services.get_question_by_id(self.QUESTION_ID)) - self.assertEqual(question.question_state_data_schema_version, 29) + self.assertEqual(question.question_state_data_schema_version, 30) # Start migration job. job_id = ( diff --git a/core/domain/question_services_test.py b/core/domain/question_services_test.py index b63060550fa2..1e2180e212f8 100644 --- a/core/domain/question_services_test.py +++ b/core/domain/question_services_test.py @@ -609,3 +609,101 @@ def test_get_skills_of_question(self): skill_ids = [skill.id for skill in skills] self.assertItemsEqual( skill_ids, ['skill_1', 'skill_2']) + + +class QuestionMigrationTests(test_utils.GenericTestBase): + + def test_migrate_question_state_from_v29_to_v30(self): + answer_group = { + 'outcome': { + 'dest': 'abc', + 'feedback': { + 'content_id': 'feedback_1', + 'html': '

Feedback

' + }, + 'labelled_as_correct': True, + 'param_changes': [], + 'refresher_exploration_id': None, + 'missing_prerequisite_skill_id': None + }, + 'rule_specs': [{ + 'inputs': { + 'x': 'Test' + }, + 'rule_type': 'Contains' + }], + 'training_data': [], + 'tagged_misconception_id': None + } + question_state_dict = { + 'content': { + 'content_id': 'content_1', + 'html': 'Question 1' + }, + 'recorded_voiceovers': { + 'voiceovers_mapping': {} + }, + 'written_translations': { + 'translations_mapping': { + 'explanation': {} + } + }, + 'interaction': { + 'answer_groups': [answer_group], + 'confirmed_unclassified_answers': [], + 'customization_args': {}, + 'default_outcome': { + 'dest': None, + 'feedback': { + 'content_id': 'feedback_1', + 'html': 'Correct Answer' + }, + 'param_changes': [], + 'refresher_exploration_id': None, + 'labelled_as_correct': True, + 'missing_prerequisite_skill_id': None + }, + 'hints': [{ + 'hint_content': { + 'content_id': 'hint_1', + 'html': 'Hint 1' + } + }], + 'solution': { + 'correct_answer': 'This is the correct answer', + 'answer_is_exclusive': False, + 'explanation': { + 'content_id': 'explanation_1', + 'html': 'Solution explanation' + } + }, + 'id': 'TextInput' + }, + 'param_changes': [], + 'solicit_answer_details': False, + 'classifier_model_id': None + } + question_model = question_models.QuestionModel( + id='question_id', + question_state_data=question_state_dict, + language_code='en', + version=0, + linked_skill_ids=['skill_id'], + question_state_data_schema_version=29) + commit_cmd = question_domain.QuestionChange({ + 'cmd': question_domain.CMD_CREATE_NEW + }) + commit_cmd_dicts = [commit_cmd.to_dict()] + question_model.commit( + 'user_id_admin', 'question model created', commit_cmd_dicts) + + current_schema_version_swap = self.swap( + feconf, 'CURRENT_STATE_SCHEMA_VERSION', 30) + + with current_schema_version_swap: + question = question_services.get_question_from_model(question_model) + + self.assertEqual(question.question_state_data_schema_version, 30) + + answer_groups = question.question_state_data.interaction.answer_groups + self.assertEqual(answer_groups[0].tagged_skill_misconception_id, None) diff --git a/core/domain/skill_services.py b/core/domain/skill_services.py index e58cccdd90c3..44e553beab4d 100644 --- a/core/domain/skill_services.py +++ b/core/domain/skill_services.py @@ -68,7 +68,7 @@ def _migrate_misconceptions_to_latest_schema(versioned_misconceptions): function to account for that new version. Args: - versioned_misconceptions: A dict with two keys: + versioned_misconceptions: dict. A dict with two keys: - schema_version: int. The schema version for the misconceptions dict. - misconceptions: list(dict). The list of dicts comprising the skill misconceptions. diff --git a/core/domain/state_domain.py b/core/domain/state_domain.py index e5a17f8debba..dc40ef772c86 100644 --- a/core/domain/state_domain.py +++ b/core/domain/state_domain.py @@ -47,7 +47,7 @@ def to_dict(self): for rule_spec in self.rule_specs], 'outcome': self.outcome.to_dict(), 'training_data': self.training_data, - 'tagged_misconception_id': self.tagged_misconception_id + 'tagged_skill_misconception_id': self.tagged_skill_misconception_id } @classmethod @@ -65,11 +65,12 @@ def from_dict(cls, answer_group_dict): Outcome.from_dict(answer_group_dict['outcome']), [RuleSpec.from_dict(rs) for rs in answer_group_dict['rule_specs']], answer_group_dict['training_data'], - answer_group_dict['tagged_misconception_id'] + answer_group_dict['tagged_skill_misconception_id'] ) def __init__( - self, outcome, rule_specs, training_data, tagged_misconception_id): + self, outcome, rule_specs, training_data, + tagged_skill_misconception_id): """Initializes a AnswerGroup domain object. Args: @@ -77,9 +78,12 @@ def __init__( rule_specs: list(RuleSpec). List of rule specifications. training_data: list(*). List of answers belonging to training data of this answer group. - tagged_misconception_id: int or None. The id of the tagged - misconception for the answer group, when a state is part of a - Question object that tests a particular skill. + tagged_skill_misconception_id: str or None. The format is + '-', where skill_id is the skill ID + of the tagged misconception and misconeption_id is the id of + the tagged misconception for the answer group. It is not None + only when a state is part of a Question object that + tests a particular skill. """ self.rule_specs = [RuleSpec( rule_spec.rule_type, rule_spec.inputs @@ -87,7 +91,7 @@ def __init__( self.outcome = outcome self.training_data = training_data - self.tagged_misconception_id = tagged_misconception_id + self.tagged_skill_misconception_id = tagged_skill_misconception_id def validate(self, interaction, exp_param_specs_dict): """Verifies that all rule classes are valid, and that the AnswerGroup @@ -110,11 +114,16 @@ def validate(self, interaction, exp_param_specs_dict): 'Expected answer group rules to be a list, received %s' % self.rule_specs) - if self.tagged_misconception_id is not None: - if not isinstance(self.tagged_misconception_id, int): + if self.tagged_skill_misconception_id is not None: + if not isinstance(self.tagged_skill_misconception_id, basestring): raise utils.ValidationError( - 'Expected tagged misconception id to be an int, ' - 'received %s' % self.tagged_misconception_id) + 'Expected tagged skill misconception id to be a str, ' + 'received %s' % self.tagged_skill_misconception_id) + if self.tagged_skill_misconception_id.count('-') != 1: + raise utils.ValidationError( + 'Expected the format of tagged skill misconception id ' + 'to be -, received %s' + % self.tagged_skill_misconception_id) if len(self.rule_specs) == 0 and len(self.training_data) == 0: raise utils.ValidationError( @@ -1540,7 +1549,7 @@ def update_interaction_answer_groups(self, answer_groups_list): answer_group = AnswerGroup( Outcome.from_dict(answer_group_dict['outcome']), [], answer_group_dict['training_data'], - answer_group_dict['tagged_misconception_id']) + answer_group_dict['tagged_skill_misconception_id']) for rule_dict in rule_specs_list: rule_spec = RuleSpec.from_dict(rule_dict) diff --git a/core/domain/state_domain_test.py b/core/domain/state_domain_test.py index cbb6ff432d26..9fa4cf3b21ae 100644 --- a/core/domain/state_domain_test.py +++ b/core/domain/state_domain_test.py @@ -674,7 +674,7 @@ def test_validate_duplicate_content_id_with_answer_groups(self): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None } exploration.init_state.update_interaction_answer_groups( @@ -1029,7 +1029,7 @@ def test_update_interaction_confirmed_unclassified_answers(self): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] self.assertEqual( @@ -1081,7 +1081,7 @@ def test_cannot_update_answer_groups_with_non_dict_rule_inputs(self): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] with self.assertRaisesRegexp( @@ -1105,7 +1105,7 @@ def test_cannot_update_answer_groups_with_non_list_rule_specs(self): }, 'rule_specs': {}, 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] with self.assertRaisesRegexp( @@ -1134,7 +1134,7 @@ def test_cannot_update_answer_groups_with_invalid_rule_input_value(self): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] with self.assertRaisesRegexp( @@ -1173,7 +1173,7 @@ def _mock_logging_function(msg, *args): 'rule_type': 'Contains' }], 'training_data': [], - 'tagged_misconception_id': None + 'tagged_skill_misconception_id': None }] exploration.init_state.update_interaction_answer_groups(answer_groups) diff --git a/core/templates/dev/head/components/question-directives/modal-templates/question-editor-modal.directive.html b/core/templates/dev/head/components/question-directives/modal-templates/question-editor-modal.directive.html index c4c1d27398af..19aa34d4dfe6 100644 --- a/core/templates/dev/head/components/question-directives/modal-templates/question-editor-modal.directive.html +++ b/core/templates/dev/head/components/question-directives/modal-templates/question-editor-modal.directive.html @@ -5,7 +5,7 @@