launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:questions-about-storm-subscriptions into launchpad:master.

Commit message:
Convert QuestionSubscription to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/386031
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:questions-about-storm-subscriptions into launchpad:master.

diff --git a/lib/lp/answers/model/question.py b/lib/lp/answers/model/question.py
index 036aeb6..682810d 100644
--- a/lib/lp/answers/model/question.py
+++ b/lib/lp/answers/model/question.py
@@ -35,6 +35,7 @@ from sqlobject import (
     StringCol,
     )
 from storm.expr import LeftJoin
+from storm.references import ReferenceSet
 from storm.store import Store
 from zope.component import getUtility
 from zope.event import notify
@@ -120,6 +121,8 @@ from lp.services.worlddata.helpers import is_english_variant
 from lp.services.worlddata.interfaces.language import ILanguage
 from lp.services.worlddata.model.language import Language
 from lp.services.xref.interfaces import IXRefSet
+from lp.services.database.interfaces import IStore
+from lp.answers.interfaces.questionsubscription import IQuestionSubscription
 
 
 class notify_question_modified:
@@ -206,11 +209,12 @@ class Question(SQLBase, BugLinkTargetMixin):
         dbName='faq', foreignKey='FAQ', notNull=False, default=None)
 
     # useful joins
-    subscriptions = SQLMultipleJoin('QuestionSubscription',
-        joinColumn='question', orderBy='id')
-    subscribers = SQLRelatedJoin('Person',
-        joinColumn='question', otherColumn='person',
-        intermediateTable='QuestionSubscription', orderBy='name')
+    subscriptions = ReferenceSet(
+        'id', 'QuestionSubscription.question_id',
+        order_by='QuestionSubscription.id')
+    subscribers = ReferenceSet(
+        'id', 'QuestionSubscription.question_id',
+        'QuestionSubscription.person_id', 'Person.id', order_by='Person.name')
     messages = SQLMultipleJoin('QuestionMessage', joinColumn='question',
         prejoins=['message'], orderBy=['QuestionMessage.id'])
     reopenings = SQLMultipleJoin('QuestionReopening', orderBy='datecreated',
@@ -261,8 +265,11 @@ class Question(SQLBase, BugLinkTargetMixin):
 
     def isSubscribed(self, person):
         """See `IQuestion`."""
-        return bool(
-            QuestionSubscription.selectOneBy(question=self, person=person))
+        store = IStore(QuestionSubscription)
+        return not store.find(
+            QuestionSubscription,
+            QuestionSubscription.question == self,
+            QuestionSubscription.person == person).is_empty()
 
     # Workflow methods
 
@@ -536,9 +543,7 @@ class Question(SQLBase, BugLinkTargetMixin):
                         '%s does not have permission to unsubscribe %s.' % (
                             unsubscribed_by.displayname,
                             person.displayname))
-                store = Store.of(sub)
-                sub.destroySelf()
-                store.flush()
+                Store.of(sub).remove(sub)
                 return
 
     def getDirectSubscribers(self):
diff --git a/lib/lp/answers/model/questionsubscription.py b/lib/lp/answers/model/questionsubscription.py
index c4e2cf7..40e3d36 100644
--- a/lib/lp/answers/model/questionsubscription.py
+++ b/lib/lp/answers/model/questionsubscription.py
@@ -8,10 +8,10 @@ __metaclass__ = type
 __all__ = ['QuestionSubscription']
 
 import pytz
-from sqlobject import ForeignKey
 from storm.locals import (
     DateTime,
     Int,
+    Reference
     )
 from zope.interface import implementer
 
@@ -19,29 +19,30 @@ from lp.answers.interfaces.questionsubscription import IQuestionSubscription
 from lp.registry.interfaces.person import validate_public_person
 from lp.registry.interfaces.role import IPersonRoles
 from lp.services.database.constants import UTC_NOW
-from lp.services.database.sqlbase import SQLBase
+from lp.services.database.stormbase import StormBase
 
 
 @implementer(IQuestionSubscription)
-class QuestionSubscription(SQLBase):
+class QuestionSubscription(StormBase):
     """A subscription for person to a question."""
 
-    _table = 'QuestionSubscription'
+    __storm_table__ = 'QuestionSubscription'
 
     id = Int(primary=True)
-    question_id = Int("question", allow_none=False)
-    question = ForeignKey(
-        dbName='question', foreignKey='Question', notNull=True)
+    question_id = Int(name="question", allow_none=False)
+    question = Reference(question_id, 'Question.id')
 
     person_id = Int(
-        "person", allow_none=False, validator=validate_public_person)
-    person = ForeignKey(
-        dbName='person', foreignKey='Person',
-        storm_validator=validate_public_person, notNull=True)
+        name="person", allow_none=False, validator=validate_public_person)
+    person = Reference(person_id, 'Person.id')
 
     date_created = DateTime(
         allow_none=False, default=UTC_NOW, tzinfo=pytz.UTC)
 
+    def __init__(self, question, person):
+        self.question = question
+        self.person = person
+
     def canBeUnsubscribedByUser(self, user):
         """See `IQuestionSubscription`."""
         if user is None: