Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1697: upgrade collection schema version to 2 by introducing language codes to collections (where the default is 'en'). #1856

Merged
merged 2 commits into from
May 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions core/controllers/home.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand All @@ -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({
Expand Down
69 changes: 59 additions & 10 deletions core/domain/collection_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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': [
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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:
Expand All @@ -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)

Expand All @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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' %
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
32 changes: 32 additions & 0 deletions core/domain/collection_domain_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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."""
Expand All @@ -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)
18 changes: 13 additions & 5 deletions core/domain/collection_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand All @@ -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,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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))
Expand Down
19 changes: 19 additions & 0 deletions core/domain/collection_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions core/domain/exp_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions core/storage/collection/gae_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions core/storage/exploration/gae_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading