Fix oppia#1697: upgrade collection schema version to 2 by introducing…

… language codes to collections (where the default is 'en'). (oppia#1856) Introduced a language_code to the collection data structure and updated its schema version to 2, where new collection have a default language code of 'en'. The support for this language code has been implemented end-to-end for collections, including in the collection creation dialog and for search queries (though this commit does not implement collection searching). * Fixed Python linter error.
anna-gavrilova · May 19, 2016 · ae6db1e · ae6db1e
1 parent 5c8b5cb
commit ae6db1e
Show file tree

Hide file tree

Showing 14 changed files with 198 additions and 28 deletions.
diff --git a/core/controllers/home.py b/core/controllers/home.py
@@ -260,7 +260,7 @@ def post(self):
         title = self.payload.get('title')
         category = self.payload.get('category')
         objective = self.payload.get('objective')
-        # TODO(bhenning): Implement support for language codes in collections.
+        language_code = self.payload.get('language_code')
 
         if not title:
             raise self.InvalidInputException('No title supplied.')
@@ -269,7 +269,8 @@ def post(self):
 
         new_collection_id = collection_services.get_new_collection_id()
         collection = collection_domain.Collection.create_default_collection(
-            new_collection_id, title, category, objective=objective)
+            new_collection_id, title, category, objective=objective,
+            language_code=language_code)
         collection_services.save_new_collection(self.user_id, collection)
 
         self.render_json({

diff --git a/core/domain/collection_domain.py b/core/domain/collection_domain.py
@@ -32,6 +32,7 @@
 COLLECTION_PROPERTY_TITLE = 'title'
 COLLECTION_PROPERTY_CATEGORY = 'category'
 COLLECTION_PROPERTY_OBJECTIVE = 'objective'
+COLLECTION_PROPERTY_LANGUAGE_CODE = 'language_code'
 COLLECTION_NODE_PROPERTY_PREREQUISITE_SKILLS = 'prerequisite_skills'
 COLLECTION_NODE_PROPERTY_ACQUIRED_SKILLS = 'acquired_skills'
 
@@ -64,7 +65,7 @@ class CollectionChange(object):
 
     COLLECTION_PROPERTIES = (
         COLLECTION_PROPERTY_TITLE, COLLECTION_PROPERTY_CATEGORY,
-        COLLECTION_PROPERTY_OBJECTIVE)
+        COLLECTION_PROPERTY_OBJECTIVE, COLLECTION_PROPERTY_LANGUAGE_CODE)
 
     def __init__(self, change_dict):
         """Initializes an CollectionChange object from a dict.
@@ -256,7 +257,7 @@ class Collection(object):
     """Domain object for an Oppia collection."""
 
     def __init__(self, collection_id, title, category, objective,
-                 schema_version, nodes, version, created_on=None,
+                 language_code, schema_version, nodes, version, created_on=None,
                  last_updated=None):
         """Constructs a new collection given all the information necessary to
         represent a collection.
@@ -277,6 +278,7 @@ def __init__(self, collection_id, title, category, objective,
         self.title = title
         self.category = category
         self.objective = objective
+        self.language_code = language_code
         self.schema_version = schema_version
         self.nodes = nodes
         self.version = version
@@ -288,6 +290,7 @@ def to_dict(self):
             'id': self.id,
             'title': self.title,
             'category': self.category,
+            'language_code': self.language_code,
             'objective': self.objective,
             'schema_version': self.schema_version,
             'nodes': [
@@ -297,9 +300,10 @@ def to_dict(self):
 
     @classmethod
     def create_default_collection(
-            cls, collection_id, title, category, objective):
+            cls, collection_id, title, category, objective,
+            language_code=feconf.DEFAULT_LANGUAGE_CODE):
         return cls(
-            collection_id, title, category, objective,
+            collection_id, title, category, objective, language_code,
             feconf.CURRENT_COLLECTION_SCHEMA_VERSION, [], 0)
 
     @classmethod
@@ -309,8 +313,9 @@ def from_dict(
         collection = cls(
             collection_dict['id'], collection_dict['title'],
             collection_dict['category'], collection_dict['objective'],
-            collection_dict['schema_version'], [], collection_version,
-            collection_created_on, collection_last_updated)
+            collection_dict['language_code'], collection_dict['schema_version'],
+            [], collection_version, collection_created_on,
+            collection_last_updated)
 
         for node_dict in collection_dict['nodes']:
             collection.nodes.append(
@@ -328,7 +333,14 @@ def to_yaml(self):
         return utils.yaml_from_dict(collection_dict)
 
     @classmethod
-    def from_yaml(cls, collection_id, yaml_content):
+    def _convert_v1_dict_to_v2_dict(cls, collection_dict):
+        """Converts a v1 collection dict into a v2 collection dict."""
+        collection_dict['schema_version'] = 2
+        collection_dict['language_code'] = feconf.DEFAULT_LANGUAGE_CODE
+        return collection_dict
+
+    @classmethod
+    def _migrate_to_latest_yaml_version(cls, yaml_content):
         try:
             collection_dict = utils.dict_from_yaml(yaml_content)
         except Exception as e:
@@ -337,6 +349,25 @@ def from_yaml(cls, collection_id, yaml_content):
                 'a zip file. The YAML parser returned the following error: %s'
                 % e)
 
+        collection_schema_version = collection_dict.get('schema_version')
+        if collection_schema_version is None:
+            raise Exception('Invalid YAML file: no schema version specified.')
+        if not (1 <= collection_schema_version
+                <= feconf.CURRENT_COLLECTION_SCHEMA_VERSION):
+            raise Exception(
+                'Sorry, we can only process v1 to v%s collection YAML files at '
+                'present.' % feconf.CURRENT_COLLECTION_SCHEMA_VERSION)
+
+        if collection_schema_version == 1:
+            collection_dict = cls._convert_v1_dict_to_v2_dict(collection_dict)
+            collection_schema_version = 2
+
+        return collection_dict
+
+    @classmethod
+    def from_yaml(cls, collection_id, yaml_content):
+        collection_dict = cls._migrate_to_latest_yaml_version(yaml_content)
+
         collection_dict['id'] = collection_id
         return Collection.from_dict(collection_dict)
 
@@ -356,8 +387,7 @@ def exploration_ids(self):
         """Returns a list of all the exploration IDs that are part of this
         collection.
         """
-        return [
-            node.exploration_id for node in self.nodes]
+        return [node.exploration_id for node in self.nodes]
 
     @property
     def init_exploration_ids(self):
@@ -415,6 +445,9 @@ def update_category(self, category):
     def update_objective(self, objective):
         self.objective = objective
 
+    def update_language_code(self, language_code):
+        self.language_code = language_code
+
     def _find_node(self, exploration_id):
         for ind, node in enumerate(self.nodes):
             if node.exploration_id == exploration_id:
@@ -471,6 +504,20 @@ def validate(self, strict=True):
             raise utils.ValidationError(
                 'An objective must be specified (in the \'Settings\' tab).')
 
+        if not isinstance(self.language_code, basestring):
+            raise utils.ValidationError(
+                'Expected language code to be a string, received %s' %
+                self.language_code)
+
+        if not self.language_code:
+            raise utils.ValidationError(
+                'A language must be specified (in the \'Settings\' tab).')
+
+        if not any([self.language_code == lc['code']
+                    for lc in feconf.ALL_LANGUAGE_CODES]):
+            raise utils.ValidationError(
+                'Invalid language code: %s' % self.language_code)
+
         if not isinstance(self.schema_version, int):
             raise utils.ValidationError(
                 'Expected schema version to be an integer, received %s' %
@@ -532,14 +579,15 @@ def validate(self, strict=True):
 class CollectionSummary(object):
     """Domain object for an Oppia collection summary."""
 
-    def __init__(self, collection_id, title, category, objective,
+    def __init__(self, collection_id, title, category, objective, language_code,
                  status, community_owned, owner_ids, editor_ids,
                  viewer_ids, contributor_ids, contributors_summary, version,
                  collection_model_created_on, collection_model_last_updated):
         self.id = collection_id
         self.title = title
         self.category = category
         self.objective = objective
+        self.language_code = language_code
         self.status = status
         self.community_owned = community_owned
         self.owner_ids = owner_ids
@@ -557,6 +605,7 @@ def to_dict(self):
             'title': self.title,
             'category': self.category,
             'objective': self.objective,
+            'language_code': self.language_code,
             'status': self.status,
             'community_owned': self.community_owned,
             'owner_ids': self.owner_ids,

diff --git a/core/domain/collection_domain_test.py b/core/domain/collection_domain_test.py
@@ -30,6 +30,7 @@
 # utils.dict_from_yaml can isolate differences quickly.
 
 SAMPLE_YAML_CONTENT = ("""category: A category
+language_code: en
 nodes:
 - acquired_skills:
   - Skill0a
@@ -82,6 +83,16 @@ def test_objective_validation(self):
         self.collection.objective = 0
         self._assert_validation_error('Expected objective to be a string')
 
+    def test_language_code_validation(self):
+        self.collection.language_code = ''
+        self._assert_validation_error('language must be specified')
+
+        self.collection.language_code = 0
+        self._assert_validation_error('Expected language code to be a string')
+
+        self.collection.language_code = 'xz'
+        self._assert_validation_error('Invalid language code')
+
     def test_schema_version_validation(self):
         self.collection.schema_version = 'some_schema_version'
         self._assert_validation_error('Expected schema version to be an int')
@@ -455,9 +466,22 @@ class SchemaMigrationUnitTests(test_utils.GenericTestBase):
 objective: ''
 schema_version: 1
 title: A title
+""")
+    YAML_CONTENT_V2 = ("""category: A category
+language_code: en
+nodes:
+- acquired_skills:
+  - Skill1
+  - Skill2
+  exploration_id: Exp1
+  prerequisite_skills: []
+objective: ''
+schema_version: 2
+title: A title
 """)
 
     _LATEST_YAML_CONTENT = YAML_CONTENT_V1
+    _LATEST_YAML_CONTENT = YAML_CONTENT_V2
 
     def test_load_from_v1(self):
         """Test direct loading from a v1 yaml file."""
@@ -466,3 +490,11 @@ def test_load_from_v1(self):
         collection = collection_domain.Collection.from_yaml(
             'cid', self.YAML_CONTENT_V1)
         self.assertEqual(collection.to_yaml(), self._LATEST_YAML_CONTENT)
+
+    def test_load_from_v2(self):
+        """Test direct loading from a v2 yaml file."""
+        self.save_new_valid_exploration(
+            'Exp1', 'user@example.com', end_state_name='End')
+        collection = collection_domain.Collection.from_yaml(
+            'cid', self.YAML_CONTENT_V2)
+        self.assertEqual(collection.to_yaml(), self._LATEST_YAML_CONTENT)
diff --git a/core/domain/collection_services.py b/core/domain/collection_services.py
@@ -125,6 +125,7 @@ def get_collection_from_model(collection_model, run_conversion=True):
     return collection_domain.Collection(
         collection_model.id, collection_model.title,
         collection_model.category, collection_model.objective,
+        collection_model.language_code,
         versioned_collection['schema_version'], [
             collection_domain.CollectionNode.from_dict(collection_node_dict)
             for collection_node_dict in versioned_collection['nodes']
@@ -137,7 +138,7 @@ def get_collection_summary_from_model(collection_summary_model):
     return collection_domain.CollectionSummary(
         collection_summary_model.id, collection_summary_model.title,
         collection_summary_model.category, collection_summary_model.objective,
-        collection_summary_model.status,
+        collection_summary_model.language_code, collection_summary_model.status,
         collection_summary_model.community_owned,
         collection_summary_model.owner_ids,
         collection_summary_model.editor_ids,
@@ -533,6 +534,9 @@ def apply_change_list(collection_id, change_list):
                 elif (change.property_name ==
                       collection_domain.COLLECTION_PROPERTY_OBJECTIVE):
                     collection.update_objective(change.new_value)
+                elif (change.property_name ==
+                      collection_domain.COLLECTION_PROPERTY_LANGUAGE_CODE):
+                    collection.update_language_code(change.new_value)
             elif (
                     change.cmd ==
                     collection_domain.CMD_MIGRATE_SCHEMA_TO_LATEST_VERSION):
@@ -618,6 +622,7 @@ def _save_collection(committer_id, collection, commit_message, change_list):
     collection_model.category = collection.category
     collection_model.title = collection.title
     collection_model.objective = collection.objective
+    collection_model.language_code = collection.language_code
     collection_model.schema_version = collection.schema_version
     collection_model.nodes = [
         collection_node.to_dict() for collection_node in collection.nodes
@@ -645,6 +650,7 @@ def _create_collection(committer_id, collection, commit_message, commit_cmds):
         category=collection.category,
         title=collection.title,
         objective=collection.objective,
+        language_code=collection.language_code,
         schema_version=collection.schema_version,
         nodes=[
             collection_node.to_dict() for collection_node in collection.nodes
@@ -820,10 +826,10 @@ def compute_summary_of_collection(collection, contributor_id_to_add):
 
     collection_summary = collection_domain.CollectionSummary(
         collection.id, collection.title, collection.category,
-        collection.objective, collection_rights.status,
-        collection_rights.community_owned, collection_rights.owner_ids,
-        collection_rights.editor_ids, collection_rights.viewer_ids,
-        contributor_ids, contributors_summary,
+        collection.objective, collection.language_code,
+        collection_rights.status, collection_rights.community_owned,
+        collection_rights.owner_ids, collection_rights.editor_ids,
+        collection_rights.viewer_ids, contributor_ids, contributors_summary,
         collection.version, collection_model_created_on,
         collection_model_last_updated
     )
@@ -866,6 +872,7 @@ def save_collection_summary(collection_summary):
         title=collection_summary.title,
         category=collection_summary.category,
         objective=collection_summary.objective,
+        language_code=collection_summary.language_code,
         status=collection_summary.status,
         community_owned=collection_summary.community_owned,
         owner_ids=collection_summary.owner_ids,
@@ -1045,6 +1052,7 @@ def _collection_to_search_dict(collection):
         'title': collection.title,
         'category': collection.category,
         'objective': collection.objective,
+        'language_code': collection.language_code,
         'rank': _get_search_rank(collection.id),
     }
     doc.update(_collection_rights_to_search_dict(rights))

diff --git a/core/domain/collection_services_test.py b/core/domain/collection_services_test.py
@@ -863,6 +863,25 @@ def test_update_collection_objective(self):
             self.COLLECTION_ID)
         self.assertEqual(collection.objective, 'Some new objective')
 
+    def test_update_collection_language_code(self):
+        # Verify initial language code.
+        collection = collection_services.get_collection_by_id(
+            self.COLLECTION_ID)
+        self.assertEqual(collection.language_code, 'en')
+
+        # Update the language code.
+        collection_services.update_collection(
+            self.owner_id, self.COLLECTION_ID, [{
+                'cmd': collection_domain.CMD_EDIT_COLLECTION_PROPERTY,
+                'property_name': 'language_code',
+                'new_value': 'fi'
+            }], 'Changed the language to Finnish')
+
+        # Verify the language is different.
+        collection = collection_services.get_collection_by_id(
+            self.COLLECTION_ID)
+        self.assertEqual(collection.language_code, 'fi')
+
     def test_update_collection_node_prerequisite_skills(self):
         # Verify initial prerequisite skills are empty.
         collection = collection_services.get_collection_by_id(

diff --git a/core/domain/exp_domain.py b/core/domain/exp_domain.py
@@ -2413,8 +2413,8 @@ def _migrate_to_latest_yaml_version(
         if not (1 <= exploration_schema_version
                 <= cls.CURRENT_EXP_SCHEMA_VERSION):
             raise Exception(
-                'Sorry, we can only process v1 to v%s YAML files at '
-                'present.' % cls.CURRENT_EXP_SCHEMA_VERSION)
+                'Sorry, we can only process v1 to v%s exploration YAML files '
+                'at present.' % cls.CURRENT_EXP_SCHEMA_VERSION)
         if exploration_schema_version == 1:
             exploration_dict = cls._convert_v1_dict_to_v2_dict(
                 exploration_dict)

diff --git a/core/storage/collection/gae_models.py b/core/storage/collection/gae_models.py
@@ -51,6 +51,9 @@ class CollectionModel(base_models.VersionedModel):
     category = ndb.StringProperty(required=True, indexed=True)
     # The objective of this collection.
     objective = ndb.TextProperty(default='', indexed=False)
+    # The language code of this collection.
+    language_code = ndb.StringProperty(
+        default=feconf.DEFAULT_LANGUAGE_CODE, indexed=True)
 
     # The version of all property blob schemas.
     schema_version = ndb.IntegerProperty(
@@ -278,6 +281,8 @@ class CollectionSummaryModel(base_models.BaseModel):
     category = ndb.StringProperty(required=True, indexed=True)
     # The objective of this collection.
     objective = ndb.TextProperty(required=True, indexed=False)
+    # The ISO 639-1 code for the language this collection is written in.
+    language_code = ndb.StringProperty(required=True, indexed=True)
 
     # Aggregate user-assigned ratings of the collection
     ratings = ndb.JsonProperty(default=None, indexed=False)

diff --git a/core/storage/exploration/gae_models.py b/core/storage/exploration/gae_models.py
@@ -310,8 +310,7 @@ class ExpSummaryModel(base_models.BaseModel):
     # The objective of this exploration.
     objective = ndb.TextProperty(required=True, indexed=False)
     # The ISO 639-1 code for the language this exploration is written in.
-    language_code = ndb.StringProperty(
-        required=True, indexed=True)
+    language_code = ndb.StringProperty(required=True, indexed=True)
     # Tags associated with this exploration.
     tags = ndb.StringProperty(repeated=True, indexed=True)