launchpad-reviewers team mailing list archive

[Colin Watson <mp+426649@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:stormify-answercontact into launchpad:master.

Commit message:
Convert AnswerContact to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/426649
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormify-answercontact into launchpad:master.

diff --git a/lib/lp/answers/model/answercontact.py b/lib/lp/answers/model/answercontact.py
index c29e43a..8b63cd9 100644
--- a/lib/lp/answers/model/answercontact.py
+++ b/lib/lp/answers/model/answercontact.py
@@ -1,38 +1,43 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""SQLBase implementation of  IAnswerContact."""
+"""ORM implementation of IAnswerContact."""
 
 __all__ = ["AnswerContact"]
 
-
+from storm.locals import Int, Reference
 from zope.interface import implementer
 
 from lp.answers.interfaces.answercontact import IAnswerContact
 from lp.registry.interfaces.person import validate_public_person
-from lp.services.database.sqlbase import SQLBase
-from lp.services.database.sqlobject import ForeignKey
+from lp.services.database.stormbase import StormBase
 
 
 @implementer(IAnswerContact)
-class AnswerContact(SQLBase):
+class AnswerContact(StormBase):
     """An entry for an answer contact for an `IQuestionTarget`."""
 
-    _defaultOrder = ["id"]
-    _table = "AnswerContact"
+    __storm_table__ = "AnswerContact"
+    __storm_order__ = ["id"]
 
-    person = ForeignKey(
-        dbName="person",
-        notNull=True,
-        foreignKey="Person",
-        storm_validator=validate_public_person,
-    )
-    product = ForeignKey(dbName="product", notNull=False, foreignKey="Product")
-    distribution = ForeignKey(
-        dbName="distribution", notNull=False, foreignKey="Distribution"
-    )
-    sourcepackagename = ForeignKey(
-        dbName="sourcepackagename",
-        notNull=False,
-        foreignKey="SourcePackageName",
+    id = Int(primary=True)
+
+    person_id = Int(
+        name="person", allow_none=False, validator=validate_public_person
     )
+    person = Reference(person_id, "Person.id")
+    product_id = Int(name="product", allow_none=True)
+    product = Reference(product_id, "Product.id")
+    distribution_id = Int(name="distribution", allow_none=True)
+    distribution = Reference(distribution_id, "Distribution.id")
+    sourcepackagename_id = Int(name="sourcepackagename", allow_none=True)
+    sourcepackagename = Reference(sourcepackagename_id, "SourcePackageName.id")
+
+    def __init__(
+        self, person, product=None, distribution=None, sourcepackagename=None
+    ):
+        super().__init__()
+        self.person = person
+        self.product = product
+        self.distribution = distribution
+        self.sourcepackagename = sourcepackagename
diff --git a/lib/lp/answers/model/question.py b/lib/lp/answers/model/question.py
index 186c7a4..b79c58c 100644
--- a/lib/lp/answers/model/question.py
+++ b/lib/lp/answers/model/question.py
@@ -1675,7 +1675,7 @@ class QuestionTargetMixin:
             LeftJoin(Person, AnswerContact.person == Person.id),
             LeftJoin(
                 PersonLanguage,
-                AnswerContact.personID == PersonLanguage.person_id,
+                AnswerContact.person == PersonLanguage.person_id,
             ),
             LeftJoin(Language, PersonLanguage.language_id == Language.id),
         ]
@@ -1710,8 +1710,10 @@ class QuestionTargetMixin:
         """See `IQuestionTarget`."""
         if not self.canUserAlterAnswerContact(person, subscribed_by):
             return False
-        answer_contact = AnswerContact.selectOneBy(
-            person=person, **self.getTargetTypes()
+        answer_contact = (
+            IStore(AnswerContact)
+            .find(AnswerContact, person=person, **self.getTargetTypes())
+            .one()
         )
         if answer_contact is not None:
             return False
@@ -1784,14 +1786,14 @@ class QuestionTargetMixin:
             return False
         if person not in self.answer_contacts:
             return False
-        answer_contact = AnswerContact.selectOneBy(
-            person=person, **self.getTargetTypes()
+        answer_contact = (
+            IStore(AnswerContact)
+            .find(AnswerContact, person=person, **self.getTargetTypes())
+            .one()
         )
         if answer_contact is None:
             return False
-        store = Store.of(answer_contact)
-        answer_contact.destroySelf()
-        store.flush()
+        Store.of(answer_contact).remove(answer_contact)
         return True
 
     def getSupportedLanguages(self):
diff --git a/lib/lp/answers/model/questionsperson.py b/lib/lp/answers/model/questionsperson.py
index dbb3107..59e2e5e 100644
--- a/lib/lp/answers/model/questionsperson.py
+++ b/lib/lp/answers/model/questionsperson.py
@@ -84,7 +84,7 @@ class QuestionsPersonMixin:
             IStore(AnswerContact)
             .find(
                 AnswerContact,
-                AnswerContact.personID == TeamParticipation.teamID,
+                AnswerContact.person == TeamParticipation.teamID,
                 TeamParticipation.person == self,
                 AnswerContact.person != self,
             )
diff --git a/lib/lp/registry/tests/test_person.py b/lib/lp/registry/tests/test_person.py
index 586cf1e..bda029d 100644
--- a/lib/lp/registry/tests/test_person.py
+++ b/lib/lp/registry/tests/test_person.py
@@ -991,7 +991,7 @@ class TestPersonStates(TestCaseWithFactory):
             warty_team)
 
     def test_AnswerContact_person_validator(self):
-        answer_contact = AnswerContact.select(limit=1)[0]
+        answer_contact = IStore(AnswerContact).find(AnswerContact).first()
         self.assertRaises(
             PrivatePersonLinkageError,
             setattr, answer_contact, 'person', self.myteam)