Merge lp:~stefanor/ibid/empty-factoid-606065 into lp:~ibid-core/ibid/old-trunk-1.6

[Stefano Rivera]

Get rid of (and disallow) empty factoid names

Get rid of (and disallow) empty factoid names

[Max Rabkin (max-rabkin)]

Can we not add a non-empty constraint to the schema so that this gets caught even if we add more ways to add factoids?

review: Approve

[Stefano Rivera (stefanor)]

> Can we not add a non-empty constraint to the schema so that this gets caught
> even if we add more ways to add factoids?

We don't currently have database-agnostic support for such constraints

[Max Rabkin (max-rabkin)]

<Taejo> tibid: ?! is <reply>$who: ‽
<tibid> Taejo: Sorry, I'm not interested in empty factoids

[Stefano Rivera (stefanor)]

Max: Of course that's a positive result from this merge, that is a separate bug

[Max Rabkin (max-rabkin)]

Of course. I was short on sleep. I'm a little concerned about the data-loss issue in the schema-upgrade. Perhaps we should rename it to "<empty>" or somesuch?

review: Approve

[Jonathan Hitchcock (vhata)]

review: Approve

=== modified file 'ibid/plugins/factoid.py'
--- ibid/plugins/factoid.py	2010-05-11 22:31:27 +0000
+++ ibid/plugins/factoid.py	2010-08-15 10:08:45 +0000
@@ -92,8 +92,15 @@
                                      key='_name', nullable=False, unique=True,
                                      index=True), force_rebuild=True)
             self.add_index(self.table.c._name)
+        def upgrade_8_to_9(self):
+            for row in self.upgrade_session.query(FactoidName) \
+                    .filter_by(name=u'') \
+                    .all():
+                self.upgrade_session.delete(row)
+                if len(row.factoid.names) == 0:
+                    self.upgrade_session.delete(row.factoid)
 
-    __table__.versioned_schema = FactoidNameSchema(__table__, 8)
+    __table__.versioned_schema = FactoidNameSchema(__table__, 9)
 
     def __init__(self, name, identity_id, factoid_id=None, factpack=None):
         self.name = name
@@ -354,6 +361,10 @@
 
         target = strip_name(target)
 
+        if target == u'':
+            event.addresponse(u"Sorry, I'm not interested in empty factoids")
+            return
+
         if target.lower() == source.lower():
             event.addresponse(u"That makes no sense, they *are* the same")
             return
@@ -541,6 +552,10 @@
 
         name = strip_name(name)
 
+        if name == u'':
+            event.addresponse(u"Sorry, I'm not interested in empty factoids")
+            return
+
         if name.lower() in self.interrogatives:
             event.addresponse(choice((
                 u"I'm afraid I have no idea",