launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:stormify-personnotification into launchpad:master.

Commit message:
Convert PersonNotification to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/386680

Convert PersonNotification to Storm
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:stormify-personnotification into launchpad:master.

diff --git a/lib/lp/registry/model/personnotification.py b/lib/lp/registry/model/personnotification.py
index 1fbc987..7a2fc29 100644
--- a/lib/lp/registry/model/personnotification.py
+++ b/lib/lp/registry/model/personnotification.py
@@ -12,10 +12,13 @@ __all__ = [
 from datetime import datetime
 
 import pytz
-from sqlobject import (
-    ForeignKey,
-    StringCol,
+from storm.base import Storm
+from storm.locals import (
+    DateTime,
+    Int,
+    Unicode,
     )
+from storm.references import Reference
 from zope.interface import implementer
 
 from lp.registry.interfaces.personnotification import (
@@ -24,11 +27,7 @@ from lp.registry.interfaces.personnotification import (
     )
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
-from lp.services.database.datetimecol import UtcDateTimeCol
-from lp.services.database.sqlbase import (
-    SQLBase,
-    sqlvalues,
-    )
+from lp.services.database.interfaces import IStore
 from lp.services.mail.sendmail import (
     format_address,
     simple_sendmail,
@@ -37,14 +36,26 @@ from lp.services.propertycache import cachedproperty
 
 
 @implementer(IPersonNotification)
-class PersonNotification(SQLBase):
+class PersonNotification(Storm):
     """See `IPersonNotification`."""
 
-    person = ForeignKey(dbName='person', notNull=True, foreignKey='Person')
-    date_created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
-    date_emailed = UtcDateTimeCol(notNull=False)
-    body = StringCol(notNull=True)
-    subject = StringCol(notNull=True)
+    __storm_table__ = 'PersonNotification'
+    id = Int(primary=True)
+    personID = Int('person')
+    person = Reference(personID, "Person.id")
+
+    date_created = DateTime(tzinfo=pytz.UTC, name='date_created',
+                            allow_none=False, default=UTC_NOW)
+    date_emailed = DateTime(tzinfo=pytz.UTC, name='date_emailed',
+                            allow_none=True)
+
+    body = Unicode(name='body', allow_none=False)
+    subject = Unicode(name='subject', allow_none=False)
+
+    def __init__(self, person, subject, body):
+        self.person = person
+        self.subject = subject
+        self.body = body
 
     @cachedproperty
     def to_addresses(self):
@@ -81,8 +92,12 @@ class PersonNotificationSet:
 
     def getNotificationsToSend(self):
         """See `IPersonNotificationSet`."""
-        return PersonNotification.selectBy(
-            date_emailed=None, orderBy=['date_created,id'])
+        store = IStore(PersonNotification)
+        return store.find(
+            PersonNotification,
+            PersonNotification.date_emailed == None).order_by(
+            PersonNotification.date_created,
+            PersonNotification.id)
 
     def addNotification(self, person, subject, body):
         """See `IPersonNotificationSet`."""
@@ -90,5 +105,7 @@ class PersonNotificationSet:
 
     def getNotificationsOlderThan(self, time_limit):
         """See `IPersonNotificationSet`."""
-        return PersonNotification.select(
-            'date_created < %s' % sqlvalues(time_limit))
+        store = IStore(PersonNotification)
+        return store.find(
+            PersonNotification,
+            PersonNotification.date_created < time_limit)
diff --git a/lib/lp/registry/scripts/personnotification.py b/lib/lp/registry/scripts/personnotification.py
index 0ba99c1..0d1e159 100644
--- a/lib/lp/registry/scripts/personnotification.py
+++ b/lib/lp/registry/scripts/personnotification.py
@@ -14,6 +14,7 @@ from datetime import (
     )
 
 import pytz
+from storm.store import Store
 from zope.component import getUtility
 
 from lp.registry.interfaces.personnotification import IPersonNotificationSet
@@ -74,9 +75,9 @@ class PersonNotificationManager:
             self.logger.info(
                 "Deleting %d old notification(s)." % to_delete.count())
             for notification in to_delete:
-                notification.destroySelf()
+                Store.of(notification).remove(notification)
             self.txn.commit()
         if extra_notifications is not None:
             for notification in extra_notifications:
-                notification.destroySelf()
+                Store.of(notification).remove(notification)
                 self.txn.commit()
diff --git a/lib/lp/registry/tests/test_personnotification.py b/lib/lp/registry/tests/test_personnotification.py
index bc7008d..10eaf62 100644
--- a/lib/lp/registry/tests/test_personnotification.py
+++ b/lib/lp/registry/tests/test_personnotification.py
@@ -40,7 +40,7 @@ class TestPersonNotification(TestCaseWithFactory):
         # The to_addresses list is the user's preferred email address.
         user = self.factory.makePerson()
         notification = self.notification_set.addNotification(
-            user, 'subject', 'body')
+            user, u'subject', u'body')
         email = '%s <%s>' % (
             user.displayname, removeSecurityProxy(user.preferredemail).email)
         self.assertEqual([email], notification.to_addresses)
@@ -51,7 +51,7 @@ class TestPersonNotification(TestCaseWithFactory):
         user = self.factory.makePerson()
         user.setPreferredEmail(None)
         notification = self.notification_set.addNotification(
-            user, 'subject', 'body')
+            user, u'subject', u'body')
         self.assertEqual([], notification.to_addresses)
         self.assertFalse(notification.can_send)
 
@@ -59,7 +59,7 @@ class TestPersonNotification(TestCaseWithFactory):
         # The to_addresses list is the team admin addresses.
         team = self.factory.makeTeam()
         notification = self.notification_set.addNotification(
-            team, 'subject', 'body')
+            team, u'subject', u'body')
         email = removeSecurityProxy(team.teamowner.preferredemail).email
         self.assertEqual([email], notification.to_addresses)
         self.assertTrue(notification.can_send)
@@ -84,7 +84,7 @@ class TestPersonNotificationManager(TestCaseWithFactory):
     def test_sendNotifications_sent(self):
         user = self.factory.makePerson()
         notification = self.notification_set.addNotification(
-            user, 'subject', 'body')
+            user, u'subject', u'body')
         unsent = self.manager.sendNotifications()
         self.assertEqual(None, unsent)
         self.assertIsNotNone(notification.date_emailed)
@@ -92,7 +92,7 @@ class TestPersonNotificationManager(TestCaseWithFactory):
     def test_sendNotifications_unsent(self):
         user = self.factory.makePerson()
         notification = self.notification_set.addNotification(
-            user, 'subject', 'body')
+            user, u'subject', u'body')
         user.setPreferredEmail(None)
         unsent = self.manager.sendNotifications()
         self.assertEqual([notification], unsent)
@@ -102,7 +102,7 @@ class TestPersonNotificationManager(TestCaseWithFactory):
         team = self.factory.makeTeam()
         self.assertIs(None, team.preferredemail)
         notification = self.notification_set.addNotification(
-            team, 'subject', 'body')
+            team, u'subject', u'body')
         unsent = self.manager.sendNotifications()
         self.assertEqual(None, unsent)
         self.assertIsNotNone(notification.date_emailed)
@@ -110,7 +110,7 @@ class TestPersonNotificationManager(TestCaseWithFactory):
     def test_purgeNotifications_old(self):
         user = self.factory.makePerson()
         notification = self.notification_set.addNotification(
-            user, 'subject', 'body')
+            user, u'subject', u'body')
         age = timedelta(
             days=int(config.person_notification.retained_days) + 1)
         naked_notification = removeSecurityProxy(notification)
@@ -123,7 +123,7 @@ class TestPersonNotificationManager(TestCaseWithFactory):
     def test_purgeNotifications_extra(self):
         user = self.factory.makePerson()
         notification = self.notification_set.addNotification(
-            user, 'subject', 'body')
+            user, u'subject', u'body')
         user.setPreferredEmail(None)
         self.manager.purgeNotifications(extra_notifications=[notification])
         notifcations = self.notification_set.getNotificationsToSend()