Fixed the backend issues with some new structure models (oppia#6514)

* Added commit log entry for topics and skills rights * Fixed backend model issues * fixed backend errors * lint
kevintab95 · Mar 26, 2019 · 3af3b04 · 3af3b04
1 parent f0c8091
commit 3af3b04
Show file tree

Hide file tree

Showing 21 changed files with 292 additions and 42 deletions.
diff --git a/core/controllers/question_editor.py b/core/controllers/question_editor.py
@@ -49,7 +49,7 @@ def post(self, skill_id):
                 (question_dict['version'] != 1)):
             raise self.InvalidInputException
 
-        question_dict['question_state_schema_version'] = (
+        question_dict['question_state_data_schema_version'] = (
             feconf.CURRENT_STATES_SCHEMA_VERSION)
         question_dict['id'] = question_services.get_new_question_id()
         try:

diff --git a/core/controllers/suggestion_test.py b/core/controllers/suggestion_test.py
@@ -389,7 +389,7 @@ def setUp(self):
             'question_state_data': self._create_valid_question_data(
                 'default_state').to_dict(),
             'language_code': 'en',
-            'question_state_schema_version': (
+            'question_state_data_schema_version': (
                 feconf.CURRENT_STATES_SCHEMA_VERSION)
         }
         self.login(self.AUTHOR_EMAIL)

diff --git a/core/domain/question_domain.py b/core/domain/question_domain.py
@@ -120,15 +120,15 @@ class Question(object):
 
     def __init__(
             self, question_id, question_state_data,
-            question_state_schema_version, language_code, version,
+            question_state_data_schema_version, language_code, version,
             created_on=None, last_updated=None):
         """Constructs a Question domain object.
 
         Args:
             question_id: str. The unique ID of the question.
             question_state_data: State. An object representing the question
                 state data.
-            question_state_schema_version: int. The schema version of the
+            question_state_data_schema_version: int. The schema version of the
                 question states (equivalent to the states schema version of
                 explorations).
             language_code: str. The ISO 639-1 code for the language this
@@ -142,7 +142,8 @@ def __init__(
         self.id = question_id
         self.question_state_data = question_state_data
         self.language_code = language_code
-        self.question_state_schema_version = question_state_schema_version
+        self.question_state_data_schema_version = (
+            question_state_data_schema_version)
         self.version = version
         self.created_on = created_on
         self.last_updated = last_updated
@@ -156,7 +157,8 @@ def to_dict(self):
         return {
             'id': self.id,
             'question_state_data': self.question_state_data.to_dict(),
-            'question_state_schema_version': self.question_state_schema_version,
+            'question_state_data_schema_version': (
+                self.question_state_data_schema_version),
             'language_code': self.language_code,
             'version': self.version
         }
@@ -209,10 +211,10 @@ def partial_validate(self):
                 'Expected language_code to be a string, received %s' %
                 self.language_code)
 
-        if not isinstance(self.question_state_schema_version, int):
+        if not isinstance(self.question_state_data_schema_version, int):
             raise utils.ValidationError(
                 'Expected schema version to be an integer, received %s' %
-                self.question_state_schema_version)
+                self.question_state_data_schema_version)
 
         if not isinstance(self.question_state_data, state_domain.State):
             raise utils.ValidationError(
@@ -286,7 +288,7 @@ def from_dict(cls, question_dict):
         question = cls(
             question_dict['id'],
             state_domain.State.from_dict(question_dict['question_state_data']),
-            question_dict['question_state_schema_version'],
+            question_dict['question_state_data_schema_version'],
             question_dict['language_code'], question_dict['version'])
 
         return question

diff --git a/core/domain/question_domain_test.py b/core/domain/question_domain_test.py
@@ -132,7 +132,7 @@ def test_to_and_from_dict(self):
         question_dict = {
             'id': 'col1.random',
             'question_state_data': default_question_state_data.to_dict(),
-            'question_state_schema_version': (
+            'question_state_data_schema_version': (
                 feconf.CURRENT_STATES_SCHEMA_VERSION),
             'language_code': 'en',
             'version': 1
@@ -218,7 +218,7 @@ def test_not_strict_validation(self):
         self._assert_validation_error(
             'Expected question state data to be a State object')
 
-        self.question.question_state_schema_version = 'abc'
+        self.question.question_state_data_schema_version = 'abc'
         self._assert_validation_error(
             'Expected schema version to be an integer')
 

diff --git a/core/domain/question_jobs_one_off.py b/core/domain/question_jobs_one_off.py
@@ -69,11 +69,11 @@ def map(item):
 
         # Write the new question into the datastore if it's different from
         # the old version.
-        if (item.question_state_schema_version <=
+        if (item.question_state_data_schema_version <=
                 feconf.CURRENT_STATES_SCHEMA_VERSION):
             commit_cmds = [question_domain.QuestionChange({
                 'cmd': question_domain.CMD_MIGRATE_STATE_SCHEMA_TO_LATEST_VERSION, # pylint: disable=line-too-long
-                'from_version': item.question_state_schema_version,
+                'from_version': item.question_state_data_schema_version,
                 'to_version': feconf.CURRENT_STATES_SCHEMA_VERSION
             })]
             question_services.update_question(

diff --git a/core/domain/question_jobs_one_off_test.py b/core/domain/question_jobs_one_off_test.py
@@ -56,7 +56,7 @@ def test_migration_job_does_not_convert_up_to_date_question(self):
         question = (
             question_services.get_question_by_id(self.QUESTION_ID))
         self.assertEqual(
-            question.question_state_schema_version,
+            question.question_state_data_schema_version,
             feconf.CURRENT_STATES_SCHEMA_VERSION)
 
         # Start migration job.
@@ -70,7 +70,7 @@ def test_migration_job_does_not_convert_up_to_date_question(self):
         updated_question = (
             question_services.get_question_by_id(self.QUESTION_ID))
         self.assertEqual(
-            updated_question.question_state_schema_version,
+            updated_question.question_state_data_schema_version,
             feconf.CURRENT_STATES_SCHEMA_VERSION)
         self.assertEqual(question.question_state_data.to_dict(),
                          updated_question.question_state_data.to_dict())
@@ -121,7 +121,7 @@ def test_migration_job_converts_old_question(self):
             self.QUESTION_ID, self.albert_id)
         question = (
             question_services.get_question_by_id(self.QUESTION_ID))
-        self.assertEqual(question.question_state_schema_version, 27)
+        self.assertEqual(question.question_state_data_schema_version, 27)
 
         # Start migration job.
         with self.swap(constants, 'ENABLE_NEW_STRUCTURE_EDITORS', True):
@@ -134,7 +134,7 @@ def test_migration_job_converts_old_question(self):
         updated_question = (
             question_services.get_question_by_id(self.QUESTION_ID))
         self.assertEqual(
-            updated_question.question_state_schema_version,
+            updated_question.question_state_data_schema_version,
             feconf.CURRENT_STATES_SCHEMA_VERSION)
 
         output = question_jobs_one_off.QuestionMigrationOneOffJob.get_output(job_id) # pylint: disable=line-too-long

diff --git a/core/domain/question_services.py b/core/domain/question_services.py
@@ -77,7 +77,8 @@ def _create_new_question(committer_id, question, commit_message):
         question_state_data=question.question_state_data.to_dict(),
         language_code=question.language_code,
         version=question.version,
-        question_state_schema_version=question.question_state_schema_version
+        question_state_data_schema_version=(
+            question.question_state_data_schema_version)
     )
     model.commit(
         committer_id, commit_message, [{'cmd': question_domain.CMD_CREATE_NEW}])
@@ -193,13 +194,14 @@ def get_question_from_model(question_model):
 
     # Ensure the original question model does not get altered.
     versioned_question_state = {
-        'state_schema_version': question_model.question_state_schema_version,
+        'state_schema_version': (
+            question_model.question_state_data_schema_version),
         'state': copy.deepcopy(
             question_model.question_state_data)
     }
 
     # Migrate the question if it is not using the latest schema version.
-    if (question_model.question_state_schema_version !=
+    if (question_model.question_state_data_schema_version !=
             feconf.CURRENT_STATES_SCHEMA_VERSION):
         _migrate_state_schema(versioned_question_state)
 
@@ -459,8 +461,8 @@ def _save_question(committer_id, question, change_list, commit_message):
     question_model = question_models.QuestionModel.get(question.id)
     question_model.question_state_data = question.question_state_data.to_dict()
     question_model.language_code = question.language_code
-    question_model.question_state_schema_version = (
-        question.question_state_schema_version)
+    question_model.question_state_data_schema_version = (
+        question.question_state_data_schema_version)
     change_dicts = [change.to_dict() for change in change_list]
     question_model.commit(committer_id, commit_message, change_dicts)
     question.version += 1

diff --git a/core/domain/subtopic_page_domain.py b/core/domain/subtopic_page_domain.py
@@ -245,21 +245,24 @@ class SubtopicPage(object):
 
     def __init__(
             self, subtopic_page_id, topic_id, page_contents,
-            language_code, version):
+            page_contents_schema_version, language_code, version):
         """Constructs a SubtopicPage domain object.
 
         Args:
             subtopic_page_id: str. The unique ID of the subtopic page.
             topic_id: str. The ID of the topic that this subtopic is a part of.
             page_contents: SubtopicPageContents. The html and audio
                 translations to be surfaced to the learner.
+            page_contents_schema_version: int. The schema version for the page
+                contents object.
             language_code: str. The ISO 639-1 code for the language this
                 subtopic page is written in.
             version: int. The current version of the subtopic.
         """
         self.id = subtopic_page_id
         self.topic_id = topic_id
         self.page_contents = page_contents
+        self.page_contents_schema_version = page_contents_schema_version
         self.language_code = language_code
         self.version = version
 
@@ -273,6 +276,7 @@ def to_dict(self):
             'id': self.id,
             'topic_id': self.topic_id,
             'page_contents': self.page_contents.to_dict(),
+            'page_contents_schema_version': self.page_contents_schema_version,
             'language_code': self.language_code,
             'version': self.version
         }
@@ -307,8 +311,33 @@ def create_default_subtopic_page(cls, subtopic_id, topic_id):
         return cls(
             subtopic_page_id, topic_id,
             SubtopicPageContents.create_default_subtopic_page_contents(),
+            feconf.CURRENT_SUBTOPIC_PAGE_CONTENTS_SCHEMA_VERSION,
             constants.DEFAULT_LANGUAGE_CODE, 0)
 
+    @classmethod
+    def update_page_contents_from_model(
+            cls, versioned_page_contents, current_version):
+        """Converts the page_contents blob contained in the given
+        versioned_skill_contents dict from current_version to
+        current_version + 1. Note that the versioned_skill_contents being
+        passed in is modified in-place.
+
+        Args:
+            versioned_page_contents: dict. A dict with two keys:
+                - schema_version: str. The schema version for the
+                    page_contents dict.
+                - page_contents: dict. The dict comprising the subtopic page
+                    contents.
+            current_version: int. The current schema version of page_contents.
+        """
+        versioned_page_contents['schema_version'] = current_version + 1
+
+        conversion_fn = getattr(
+            cls, '_convert_page_contents_v%s_dict_to_v%s_dict' % (
+                current_version, current_version + 1))
+        versioned_page_contents['skill_contents'] = conversion_fn(
+            versioned_page_contents['skill_contents'])
+
     def get_subtopic_id_from_subtopic_page_id(self):
         """Returns the id from the subtopic page id of the object.
 
@@ -373,6 +402,20 @@ def validate(self):
                 self.version)
         self.page_contents.validate()
 
+        if not isinstance(self.page_contents_schema_version, int):
+            raise utils.ValidationError(
+                'Expected page contents schema version to be an integer, '
+                'received %s' % self.page_contents_schema_version)
+        if (
+                self.page_contents_schema_version !=
+                feconf.CURRENT_SUBTOPIC_PAGE_CONTENTS_SCHEMA_VERSION):
+            raise utils.ValidationError(
+                'Expected page contents schema version to be %s, received %s'
+                % (
+                    feconf.CURRENT_SUBTOPIC_PAGE_CONTENTS_SCHEMA_VERSION,
+                    self.page_contents_schema_version)
+            )
+
         if not isinstance(self.language_code, basestring):
             raise utils.ValidationError(
                 'Expected language code to be a string, received %s' %

diff --git a/core/domain/subtopic_page_domain_test.py b/core/domain/subtopic_page_domain_test.py
@@ -20,6 +20,7 @@
 from core.domain import state_domain
 from core.domain import subtopic_page_domain
 from core.tests import test_utils
+import feconf
 import utils
 
 
@@ -52,6 +53,8 @@ def test_to_dict(self):
                     }
                 }
             },
+            'page_contents_schema_version': (
+                feconf.CURRENT_SUBTOPIC_PAGE_CONTENTS_SCHEMA_VERSION),
             'language_code': constants.DEFAULT_LANGUAGE_CODE,
             'version': 0
         }
@@ -81,6 +84,8 @@ def test_create_default_subtopic_page(self):
                     }
                 }
             },
+            'page_contents_schema_version': (
+                feconf.CURRENT_SUBTOPIC_PAGE_CONTENTS_SCHEMA_VERSION),
             'language_code': constants.DEFAULT_LANGUAGE_CODE,
             'version': 0
         }
@@ -138,6 +143,8 @@ def test_update_audio(self):
                     }
                 }
             },
+            'page_contents_schema_version': (
+                feconf.CURRENT_SUBTOPIC_PAGE_CONTENTS_SCHEMA_VERSION),
             'language_code': constants.DEFAULT_LANGUAGE_CODE,
             'version': 0
         }
@@ -164,6 +171,8 @@ def test_update_html(self):
                     }
                 }
             },
+            'page_contents_schema_version': (
+                feconf.CURRENT_SUBTOPIC_PAGE_CONTENTS_SCHEMA_VERSION),
             'language_code': constants.DEFAULT_LANGUAGE_CODE,
             'version': 0
         }
@@ -198,6 +207,8 @@ def test_update_written_translations(self):
                 },
                 'written_translations': written_translations_dict
             },
+            'page_contents_schema_version': (
+                feconf.CURRENT_SUBTOPIC_PAGE_CONTENTS_SCHEMA_VERSION),
             'language_code': constants.DEFAULT_LANGUAGE_CODE,
             'version': 0
         }