Add support for collection tags (oppia#2010)

* Fix incorrect collection tile spacing. * Add tags for collections. * Add a pre-publish modal to the collection editor.
anna-gavrilova · Jun 6, 2016 · 37626e5 · 37626e5
1 parent 2415890
commit 37626e5
Show file tree

Hide file tree

Showing 21 changed files with 342 additions and 60 deletions.
diff --git a/core/domain/collection_domain.py b/core/domain/collection_domain.py
@@ -22,6 +22,8 @@
 """
 
 import copy
+import re
+import string
 
 import feconf
 import utils
@@ -33,6 +35,7 @@
 COLLECTION_PROPERTY_CATEGORY = 'category'
 COLLECTION_PROPERTY_OBJECTIVE = 'objective'
 COLLECTION_PROPERTY_LANGUAGE_CODE = 'language_code'
+COLLECTION_PROPERTY_TAGS = 'tags'
 COLLECTION_NODE_PROPERTY_PREREQUISITE_SKILLS = 'prerequisite_skills'
 COLLECTION_NODE_PROPERTY_ACQUIRED_SKILLS = 'acquired_skills'
 
@@ -65,7 +68,8 @@ class CollectionChange(object):
 
     COLLECTION_PROPERTIES = (
         COLLECTION_PROPERTY_TITLE, COLLECTION_PROPERTY_CATEGORY,
-        COLLECTION_PROPERTY_OBJECTIVE, COLLECTION_PROPERTY_LANGUAGE_CODE)
+        COLLECTION_PROPERTY_OBJECTIVE, COLLECTION_PROPERTY_LANGUAGE_CODE,
+        COLLECTION_PROPERTY_TAGS)
 
     def __init__(self, change_dict):
         """Initializes an CollectionChange object from a dict.
@@ -257,8 +261,8 @@ class Collection(object):
     """Domain object for an Oppia collection."""
 
     def __init__(self, collection_id, title, category, objective,
-                 language_code, schema_version, nodes, version, created_on=None,
-                 last_updated=None):
+                 language_code, tags, schema_version, nodes, version,
+                 created_on=None, last_updated=None):
         """Constructs a new collection given all the information necessary to
         represent a collection.
 
@@ -279,6 +283,7 @@ def __init__(self, collection_id, title, category, objective,
         self.category = category
         self.objective = objective
         self.language_code = language_code
+        self.tags = tags
         self.schema_version = schema_version
         self.nodes = nodes
         self.version = version
@@ -290,8 +295,9 @@ def to_dict(self):
             'id': self.id,
             'title': self.title,
             'category': self.category,
-            'language_code': self.language_code,
             'objective': self.objective,
+            'language_code': self.language_code,
+            'tags': self.tags,
             'schema_version': self.schema_version,
             'nodes': [
                 node.to_dict() for node in self.nodes
@@ -305,7 +311,7 @@ def create_default_collection(
             objective=feconf.DEFAULT_COLLECTION_OBJECTIVE,
             language_code=feconf.DEFAULT_LANGUAGE_CODE):
         return cls(
-            collection_id, title, category, objective, language_code,
+            collection_id, title, category, objective, language_code, [],
             feconf.CURRENT_COLLECTION_SCHEMA_VERSION, [], 0)
 
     @classmethod
@@ -315,9 +321,9 @@ def from_dict(
         collection = cls(
             collection_dict['id'], collection_dict['title'],
             collection_dict['category'], collection_dict['objective'],
-            collection_dict['language_code'], collection_dict['schema_version'],
-            [], collection_version, collection_created_on,
-            collection_last_updated)
+            collection_dict['language_code'], collection_dict['tags'],
+            collection_dict['schema_version'], [], collection_version,
+            collection_created_on, collection_last_updated)
 
         for node_dict in collection_dict['nodes']:
             collection.nodes.append(
@@ -339,6 +345,7 @@ 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
+        collection_dict['tags'] = []
         return collection_dict
 
     @classmethod
@@ -450,6 +457,9 @@ def update_objective(self, objective):
     def update_language_code(self, language_code):
         self.language_code = language_code
 
+    def update_tags(self, tags):
+        self.tags = tags
+
     def _find_node(self, exploration_id):
         for ind, node in enumerate(self.nodes):
             if node.exploration_id == exploration_id:
@@ -518,6 +528,35 @@ def validate(self, strict=True):
             raise utils.ValidationError(
                 'Invalid language code: %s' % self.language_code)
 
+        if not isinstance(self.tags, list):
+            raise utils.ValidationError(
+                'Expected tags to be a list, received %s' % self.tags)
+        for tag in self.tags:
+            if not isinstance(tag, basestring):
+                raise utils.ValidationError(
+                    'Expected each tag to be a string, received \'%s\'' % tag)
+
+            if not tag:
+                raise utils.ValidationError('Tags should be non-empty.')
+
+            if not re.match(feconf.TAG_REGEX, tag):
+                raise utils.ValidationError(
+                    'Tags should only contain lowercase letters and spaces, '
+                    'received \'%s\'' % tag)
+
+            if (tag[0] not in string.ascii_lowercase or
+                    tag[-1] not in string.ascii_lowercase):
+                raise utils.ValidationError(
+                    'Tags should not start or end with whitespace, received '
+                    ' \'%s\'' % tag)
+
+            if re.search(r'\s\s+', tag):
+                raise utils.ValidationError(
+                    'Adjacent whitespace in tags should be collapsed, '
+                    'received \'%s\'' % tag)
+        if len(set(self.tags)) != len(self.tags):
+            raise utils.ValidationError('Some tags duplicate each other')
+
         if not isinstance(self.schema_version, int):
             raise utils.ValidationError(
                 'Expected schema version to be an integer, received %s' %
@@ -592,7 +631,7 @@ class CollectionSummary(object):
     """Domain object for an Oppia collection summary."""
 
     def __init__(self, collection_id, title, category, objective, language_code,
-                 status, community_owned, owner_ids, editor_ids,
+                 tags, status, community_owned, owner_ids, editor_ids,
                  viewer_ids, contributor_ids, contributors_summary, version,
                  node_count, collection_model_created_on,
                  collection_model_last_updated):
@@ -601,6 +640,7 @@ def __init__(self, collection_id, title, category, objective, language_code,
         self.category = category
         self.objective = objective
         self.language_code = language_code
+        self.tags = tags
         self.status = status
         self.community_owned = community_owned
         self.owner_ids = owner_ids
@@ -620,6 +660,7 @@ def to_dict(self):
             'category': self.category,
             'objective': self.objective,
             'language_code': self.language_code,
+            'tags': self.tags,
             '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
@@ -39,6 +39,7 @@
   prerequisite_skills: []
 objective: An objective
 schema_version: %d
+tags: []
 title: A title
 """) % (feconf.CURRENT_COLLECTION_SCHEMA_VERSION)
 
@@ -93,6 +94,31 @@ def test_language_code_validation(self):
         self.collection.language_code = 'xz'
         self._assert_validation_error('Invalid language code')
 
+    def test_tags_validation(self):
+        self.collection.tags = 'abc'
+        self._assert_validation_error('Expected tags to be a list')
+
+        self.collection.tags = [2, 3]
+        self._assert_validation_error('Expected each tag to be a string')
+
+        self.collection.tags = ['', 'tag']
+        self._assert_validation_error('Tags should be non-empty')
+
+        self.collection.tags = ['234']
+        self._assert_validation_error(
+            'Tags should only contain lowercase letters and spaces')
+
+        self.collection.tags = ['   abc']
+        self._assert_validation_error(
+            'Tags should not start or end with whitespace')
+
+        self.collection.tags = ['abc  def']
+        self._assert_validation_error(
+            'Adjacent whitespace in tags should be collapsed')
+
+        self.collection.tags = ['abc', 'abc']
+        self._assert_validation_error('Some tags duplicate each other')
+
     def test_schema_version_validation(self):
         self.collection.schema_version = 'some_schema_version'
         self._assert_validation_error('Expected schema version to be an int')
@@ -508,6 +534,7 @@ class SchemaMigrationUnitTests(test_utils.GenericTestBase):
   prerequisite_skills: []
 objective: ''
 schema_version: 2
+tags: []
 title: A title
 """)
 

diff --git a/core/domain/collection_services.py b/core/domain/collection_services.py
@@ -124,7 +124,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,
+        collection_model.language_code, collection_model.tags,
         versioned_collection['schema_version'], [
             collection_domain.CollectionNode.from_dict(collection_node_dict)
             for collection_node_dict in versioned_collection['nodes']
@@ -137,7 +137,8 @@ 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.language_code, collection_summary_model.status,
+        collection_summary_model.language_code, collection_summary_model.tags,
+        collection_summary_model.status,
         collection_summary_model.community_owned,
         collection_summary_model.owner_ids,
         collection_summary_model.editor_ids,
@@ -454,6 +455,9 @@ def apply_change_list(collection_id, change_list):
                 elif (change.property_name ==
                       collection_domain.COLLECTION_PROPERTY_LANGUAGE_CODE):
                     collection.update_language_code(change.new_value)
+                elif (change.property_name ==
+                      collection_domain.COLLECTION_PROPERTY_TAGS):
+                    collection.update_tags(change.new_value)
             elif (
                     change.cmd ==
                     collection_domain.CMD_MIGRATE_SCHEMA_TO_LATEST_VERSION):
@@ -540,6 +544,7 @@ def _save_collection(committer_id, collection, commit_message, change_list):
     collection_model.title = collection.title
     collection_model.objective = collection.objective
     collection_model.language_code = collection.language_code
+    collection_model.tags = collection.tags
     collection_model.schema_version = collection.schema_version
     collection_model.nodes = [
         collection_node.to_dict() for collection_node in collection.nodes
@@ -568,6 +573,7 @@ def _create_collection(committer_id, collection, commit_message, commit_cmds):
         title=collection.title,
         objective=collection.objective,
         language_code=collection.language_code,
+        tags=collection.tags,
         schema_version=collection.schema_version,
         nodes=[
             collection_node.to_dict() for collection_node in collection.nodes
@@ -744,7 +750,7 @@ def compute_summary_of_collection(collection, contributor_id_to_add):
 
     collection_summary = collection_domain.CollectionSummary(
         collection.id, collection.title, collection.category,
-        collection.objective, collection.language_code,
+        collection.objective, collection.language_code, collection.tags,
         collection_rights.status, collection_rights.community_owned,
         collection_rights.owner_ids, collection_rights.editor_ids,
         collection_rights.viewer_ids, contributor_ids, contributors_summary,
@@ -792,6 +798,7 @@ def save_collection_summary(collection_summary):
         category=collection_summary.category,
         objective=collection_summary.objective,
         language_code=collection_summary.language_code,
+        tags=collection_summary.tags,
         status=collection_summary.status,
         community_owned=collection_summary.community_owned,
         owner_ids=collection_summary.owner_ids,
@@ -973,6 +980,7 @@ def _collection_to_search_dict(collection):
         'category': collection.category,
         'objective': collection.objective,
         'language_code': collection.language_code,
+        'tags': collection.tags,
         '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_language_code(self):
             self.COLLECTION_ID)
         self.assertEqual(collection.language_code, 'fi')
 
+    def test_update_collection_tags(self):
+        # Verify initial tags.
+        collection = collection_services.get_collection_by_id(
+            self.COLLECTION_ID)
+        self.assertEqual(collection.tags, [])
+
+        # Update the tags.
+        collection_services.update_collection(
+            self.owner_id, self.COLLECTION_ID, [{
+                'cmd': collection_domain.CMD_EDIT_COLLECTION_PROPERTY,
+                'property_name': 'tags',
+                'new_value': ['test']
+            }], 'Add a new tag')
+
+        # Verify that the tags are different.
+        collection = collection_services.get_collection_by_id(
+            self.COLLECTION_ID)
+        self.assertEqual(collection.tags, ['test'])
+
     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/summary_services.py b/core/domain/summary_services.py
@@ -255,6 +255,8 @@ def _get_displayable_collection_summary_dicts(collection_summaries):
                 'category': collection_summary.category,
                 'activity_type': rights_manager.ACTIVITY_TYPE_COLLECTION,
                 'objective': collection_summary.objective,
+                'language_code': collection_summary.language_code,
+                'tags': collection_summary.tags,
                 'node_count': collection_summary.node_count,
                 'last_updated_msec': utils.get_time_in_millisecs(
                     collection_summary.collection_model_last_updated),

diff --git a/core/storage/collection/gae_models.py b/core/storage/collection/gae_models.py
@@ -54,6 +54,8 @@ class CollectionModel(base_models.VersionedModel):
     # The language code of this collection.
     language_code = ndb.StringProperty(
         default=feconf.DEFAULT_LANGUAGE_CODE, indexed=True)
+    # Tags associated with this collection.
+    tags = ndb.StringProperty(repeated=True, indexed=True)
 
     # The version of all property blob schemas.
     schema_version = ndb.IntegerProperty(
@@ -283,6 +285,8 @@ class CollectionSummaryModel(base_models.BaseModel):
     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)
+    # Tags associated with this collection.
+    tags = ndb.StringProperty(repeated=True, indexed=True)
 
     # Aggregate user-assigned ratings of the collection
     ratings = ndb.JsonProperty(default=None, indexed=False)