Merge pull request #253 from mozilla-services/allow-empty-article-tit…

…le-2

 Rework previous fix about empty titles

readinglist/tests/test_article_schema.py

@@ -34,7 +34,6 @@ def test_record_validation_default_values(self):
     def test_record_validation_computed_values(self):
         self.assertIsNotNone(self.deserialized.get('stored_on'))
         self.assertIsNotNone(self.deserialized.get('added_on'))
-        self.assertIsNotNone(self.deserialized.get('last_modified'))
 
     def test_url_is_required(self):
         self.record.pop('url')
@@ -65,10 +64,11 @@ def test_resolved_url_has_max_length(self):
                           self.schema.deserialize,
                           self.record)
 
-    def test_title_is_not_required(self):
+    def test_title_is_required(self):
         self.record.pop('title')
-        deserialized = self.schema.deserialize(self.record)
-        self.assertEqual(deserialized['title'], None)
+        self.assertRaises(colander.Invalid,
+                          self.schema.deserialize,
+                          self.record)
 
     def test_title_is_stripped(self):
         self.record['title'] = '  Nous Sommes Charlie  '
@@ -84,15 +84,20 @@ def test_title_has_max_length(self):
         deserialized = self.schema.deserialize(self.record)
         self.assertEqual(len(deserialized['title']), 1024)
 
-    def test_title_is_set_to_null_if_empty_string(self):
+    def test_title_allows_empty_string(self):
         self.record['title'] = ''
         deserialized = self.schema.deserialize(self.record)
+        self.assertEqual(deserialized['title'], '')
+
+    def test_title_allows_null(self):
+        self.record['title'] = None
+        deserialized = self.schema.deserialize(self.record)
         self.assertEqual(deserialized['title'], None)
 
-    def test_title_is_set_to_null_if_blank_string(self):
+    def test_title_is_set_to_empty_if_blank_string(self):
         self.record['title'] = '   '
         deserialized = self.schema.deserialize(self.record)
-        self.assertEqual(deserialized['title'], None)
+        self.assertEqual(deserialized['title'], '')
 
     def test_resolved_title_has_max_length(self):
         self.record['resolved_title'] = u'\u76d8' * 1025
@@ -104,15 +109,20 @@ def test_resolved_title_is_stripped(self):
         deserialized = self.schema.deserialize(self.record)
         self.assertEqual(deserialized['resolved_title'], 'Nous Sommes Charlie')
 
-    def test_resolved_title_is_set_to_null_if_empty_string(self):
+    def test_resolved_title_allows_empty_string(self):
         self.record['resolved_title'] = ''
         deserialized = self.schema.deserialize(self.record)
+        self.assertEqual(deserialized['resolved_title'], '')
+
+    def test_resolved_title_allows_null(self):
+        self.record['resolved_title'] = None
+        deserialized = self.schema.deserialize(self.record)
         self.assertEqual(deserialized['resolved_title'], None)
 
-    def test_resolved_title_is_set_to_null_if_blank_string(self):
+    def test_resolved_title_is_set_to_empty_if_blank_string(self):
         self.record['resolved_title'] = '   '
         deserialized = self.schema.deserialize(self.record)
-        self.assertEqual(deserialized['resolved_title'], None)
+        self.assertEqual(deserialized['resolved_title'], '')
 
     def test_added_by_is_required(self):
         self.record.pop('added_by')

readinglist/views/article.py

@@ -20,25 +20,45 @@ def preparer(self, appstruct):
         return strip_whitespace(appstruct)
 
 
+class BlankString(String):
+    def deserialize(self, node, cstruct):
+        """Override basic Colander String behaviour to deserialize empty
+        strings as such.
+
+        See https://github.com/Pylons/colander/issues/214.
+        """
+        if cstruct == '':
+            return ''
+        if cstruct is None:
+            return None
+        return super(BlankString, self).deserialize(node, cstruct)
+
+
+class NullOrLength(colander.Length):
+    def __call__(self, node, value):
+        if value not in (colander.null, None):
+            super(NullOrLength, self).__call__(node, value)
+
+
 class ArticleTitle(SchemaNode):
     """String representing the title of an article."""
-    schema_type = String
-    validator = colander.Length(max=TITLE_MAX_LENGTH)
+    schema_type = BlankString
+    validator = NullOrLength(max=TITLE_MAX_LENGTH)
 
     def preparer(self, appstruct):
         if appstruct:
             # Strip then truncate the title to TITLE_MAX_LENGTH
             appstruct = strip_whitespace(appstruct)[:TITLE_MAX_LENGTH]
 
-        return appstruct or colander.null
+        return appstruct
 
 
 class ArticleSchema(ResourceSchema):
     """Schema for a reading list article."""
 
     url = URL()
     preview = URL(missing=None)
-    title = ArticleTitle(missing=None)
+    title = ArticleTitle()
     added_by = DeviceName()
     added_on = TimeStamp()
     stored_on = TimeStamp()