Skip to content

Commit

Permalink
[GSoC'24] M2.1 - Inapplicable misconception IDs state migration (oppi…
Browse files Browse the repository at this point in the history
…a#20673)

* Fix acceptance test

* Revert accidental commit

* Write state migration for inapplicable misconception IDs

* Fix migration

* Fix migration

* Add comment

* Fix type issues in tests

* Fix lint issues

* Fix type errors

* Fix migration

* Fix backend test

* Attempt to fix backend test

* Attempt to fix backend tests

* Backend fix

* Attempt backend test fix

* Attempt backend migration fix

* Update golden zip file

* Fix backend tests

* Attempt backend test fix

* Fix backend test

* Debug backend tests

* Attempt backend test fix

* Attempt backend test fix

* Attempt backend test fix

* Fix backend coverage

* Fix backend test

* Add field to frontend object

* Fix type issue

* Fix e2e test

* Fix e2e test

* Fix e2e test

* Fix e2e test

* Update docstring

* Update docstring

* Fix docstring

* Remove unaudited validation blocking migrations

* Remove backend validation test code
  • Loading branch information
Vir-8 authored Aug 4, 2024
1 parent cbdf464 commit 7633b0f
Show file tree
Hide file tree
Showing 25 changed files with 457 additions and 79 deletions.
1 change: 1 addition & 0 deletions assets/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6287,6 +6287,7 @@ export default {
"html": "",
"content_id": "content"
},
"inapplicable_skill_misconception_ids": null,
"interaction": {
"id": null,
"customization_args": {},
Expand Down
5 changes: 3 additions & 2 deletions core/controllers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,9 @@
'content_0': {}
}
},
'solicit_answer_details': False,
'card_is_checkpoint': True,
'solicit_answer_details': False,
'card_is_checkpoint': True,
'inapplicable_skill_misconception_ids': None
}
},
'version': 3
Expand Down
6 changes: 4 additions & 2 deletions core/controllers/domain_objects_validator_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,8 @@ def test_valid_object_raises_no_exception(self) -> None:
},
'classifier_model_id': None,
'card_is_checkpoint': False,
'solicit_answer_details': False
'solicit_answer_details': False,
'inapplicable_skill_misconception_ids': None
}
domain_objects_validator.validate_state_dict(state_dict)

Expand Down Expand Up @@ -496,7 +497,8 @@ def test_valid_object_raises_no_exception(self) -> None:
}
},
'solicit_answer_details': False,
'card_is_checkpoint': False
'card_is_checkpoint': False,
'inapplicable_skill_misconception_ids': None
}
domain_objects_validator.validate_question_state_dict(
question_state_dict)
Expand Down
4 changes: 4 additions & 0 deletions core/controllers/editor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ class DownloadIntegrationTest(BaseEditorControllerTests):
content:
content_id: content_3
html: ''
inapplicable_skill_misconception_ids: null
interaction:
answer_groups: []
confirmed_unclassified_answers: []
Expand Down Expand Up @@ -430,6 +431,7 @@ class DownloadIntegrationTest(BaseEditorControllerTests):
content:
content_id: content_5
html: ''
inapplicable_skill_misconception_ids: null
interaction:
answer_groups: []
confirmed_unclassified_answers: []
Expand Down Expand Up @@ -470,6 +472,7 @@ class DownloadIntegrationTest(BaseEditorControllerTests):
content:
content_id: content_0
html: ''
inapplicable_skill_misconception_ids: null
interaction:
answer_groups: []
confirmed_unclassified_answers: []
Expand Down Expand Up @@ -512,6 +515,7 @@ class DownloadIntegrationTest(BaseEditorControllerTests):
content:
content_id: content_3
html: ''
inapplicable_skill_misconception_ids: null
interaction:
answer_groups: []
confirmed_unclassified_answers: []
Expand Down
3 changes: 2 additions & 1 deletion core/controllers/suggestion_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2383,7 +2383,8 @@ def test_accept_question_suggestion_with_image_region_interactions(
}
},
'solicit_answer_details': False,
'card_is_checkpoint': False
'card_is_checkpoint': False,
'inapplicable_skill_misconception_ids': None
}
question_dict: question_domain.QuestionSuggestionChangeDict = {
'question_state_data': question_state_dict,
Expand Down
8 changes: 5 additions & 3 deletions core/domain/caching_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class CachingServicesUnitTests(test_utils.GenericTestBase):
'objective': '',
'init_state_name': 'Introduction',
'author_notes': '',
'states_schema_version': 53,
'states_schema_version': 56,
'param_specs': {},
'param_changes': [],
'id': 'h51Bu72rDIqO',
Expand All @@ -76,6 +76,7 @@ class CachingServicesUnitTests(test_utils.GenericTestBase):
'html': '<p>Unicode Characters 😍😍😍😍</p>'
},
'linked_skill_id': None,
'inapplicable_skill_misconception_ids': None,
'interaction': {
'hints': [{
'hint_content': {
Expand Down Expand Up @@ -152,9 +153,10 @@ class CachingServicesUnitTests(test_utils.GenericTestBase):
json_encoded_string_representing_an_exploration = (
'{"param_changes": [], "category": "", "auto_tts_enabled": true, '
'"next_content_id_index": 7, "tags"'
': [], "states_schema_version": 53, "title": "", "param_specs": {}, "id'
': [], "states_schema_version": 56, "title": "", "param_specs": {}, "id'
'": "h51Bu72rDIqO", "states": {"Introduction": {"param_changes": [], "c'
'ard_is_checkpoint": true, "interaction": {"solution": null, "answer_gr'
'ard_is_checkpoint": true, "inapplicable_skill_misconception_ids": null'
', "interaction": {"solution": null, "answer_gr'
'oups": [{"tagged_skill_misconception_id": null, "outcome": {"param_cha'
'nges": [], "feedback": {"content_id": "feedback_4", "html": "<p>This i'
's great! \\u00ae\\u00ae</p>"}, "dest": "Introduction", "dest_if_really'
Expand Down
21 changes: 21 additions & 0 deletions core/domain/draft_upgrade_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,27 @@ def _convert_html_in_draft_change_list(
})
return draft_change_list

@classmethod
def _convert_states_v55_dict_to_v56_dict(
cls, draft_change_list: List[exp_domain.ExplorationChange]
) -> List[exp_domain.ExplorationChange]:
"""Converts draft change list from state version 55 to 56. Version 56
adds an inapplicable_skill_misconception_ids list property to the
state.
Args:
draft_change_list: list(ExplorationChange). The list of
ExplorationChange domain objects to upgrade.
Returns:
list(ExplorationChange). The converted draft_change_list.
Raises:
InvalidDraftConversionException. The conversion cannot be
completed.
"""
return draft_change_list

@classmethod
def _convert_states_v54_dict_to_v55_dict(
cls, draft_change_list: List[exp_domain.ExplorationChange]
Expand Down
29 changes: 29 additions & 0 deletions core/domain/draft_upgrade_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,35 @@ def test_convert_to_latest_schema_version_implemented(self) -> None:
msg='Current schema version is %d but DraftUpgradeUtil.%s is '
'unimplemented.' % (state_schema_version, conversion_fn_name))

def test_convert_states_v55_dict_to_v56_dict(self) -> None:
draft_change_list_v55 = [
exp_domain.ExplorationChange({
'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
'state_name': 'Intro',
'property_name': 'content',
'new_value': 'TextInput'
})
]
# Migrate exploration to state schema version 56.
self.create_and_migrate_new_exploration('55', '56')
migrated_draft_change_list_v56 = (
draft_upgrade_services.try_upgrading_draft_to_exp_version(
draft_change_list_v55, 1, 2, self.EXP_ID)
)
# Ruling out the possibility of None for mypy type checking.
assert migrated_draft_change_list_v56 is not None
# Change draft change lists into a list of dicts so that it is
# easy to compare the whole draft change list.
draft_change_list_v55_dict_list = [
change.to_dict() for change in draft_change_list_v55
]
migrated_draft_change_list_v56_dict_list = [
change.to_dict() for change in migrated_draft_change_list_v56
]
self.assertEqual(
draft_change_list_v55_dict_list,
migrated_draft_change_list_v56_dict_list)

def test_convert_states_v54_dict_to_v55_dict_without_state_changes(
self
) -> None:
Expand Down
57 changes: 56 additions & 1 deletion core/domain/exp_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,9 @@ def from_dict(

state.card_is_checkpoint = sdict['card_is_checkpoint']

state.inapplicable_skill_misconception_ids = (
sdict['inapplicable_skill_misconception_ids'])

exploration.states[state_name] = state

exploration.param_changes = [
Expand Down Expand Up @@ -5123,6 +5126,27 @@ def _convert_states_v54_dict_to_v55_dict(

return states_dict, next_content_id_index

@classmethod
def _convert_states_v55_dict_to_v56_dict(
cls, states_dict: Dict[str, state_domain.StateDict]
) -> Dict[str, state_domain.StateDict]:
"""Converts from v55 to v56. Version 56 adds an
inapplicable_skill_misconception_ids list to the state.
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[str, state_domain.StateDict]. The converted
v56 state dictionary.
"""
for _, state_dict in states_dict.items():
state_dict['inapplicable_skill_misconception_ids'] = []

return states_dict

@classmethod
def update_states_from_model(
cls,
Expand Down Expand Up @@ -5178,7 +5202,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 = 60
CURRENT_EXP_SCHEMA_VERSION = 61
EARLIEST_SUPPORTED_EXP_SCHEMA_VERSION = 46

@classmethod
Expand Down Expand Up @@ -5527,6 +5551,32 @@ def _convert_v59_dict_to_v60_dict(

return exploration_dict

@classmethod
def _convert_v60_dict_to_v61_dict(
cls, exploration_dict: VersionedExplorationDict
) -> VersionedExplorationDict:
"""Converts a v60 exploration dict into a v61 exploration dict.
Introduces the inapplicable_skill_misconception_ids list into
the state properties.
Args:
exploration_dict: dict. The dict representation of an exploration
with schema version v60.
Returns:
dict. The dict representation of the Exploration domain object,
following schema version v61.
"""
exploration_dict['schema_version'] = 61

exploration_dict['states'] = (
cls._convert_states_v55_dict_to_v56_dict(
exploration_dict['states'])
)
exploration_dict['states_schema_version'] = 56

return exploration_dict

@classmethod
def _migrate_to_latest_yaml_version(
cls, yaml_content: str
Expand Down Expand Up @@ -5639,6 +5689,11 @@ def _migrate_to_latest_yaml_version(
exploration_dict)
exploration_schema_version = 60

if exploration_schema_version == 60:
exploration_dict = cls._convert_v60_dict_to_v61_dict(
exploration_dict)
exploration_schema_version = 61

return exploration_dict

@classmethod
Expand Down
Loading

0 comments on commit 7633b0f

Please sign in to comment.