Merge ~jugmac00/launchpad:SignedCodeOfConduct.affirmed-cant-be-None-anymore into launchpad:master

[Jürgen Gmach]

SignedCodeOfConduct.affirmed can't be None anymore

[Colin Watson (cjwatson)]

review: Needs Fixing

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/registry/model/codeofconduct.py b/lib/lp/registry/model/codeofconduct.py
index 2c19f99..65db887 100644
--- a/lib/lp/registry/model/codeofconduct.py
+++ b/lib/lp/registry/model/codeofconduct.py
@@ -199,7 +199,7 @@ class SignedCodeOfConduct(StormBase):
 
     active = Bool(name='active', allow_none=False, default=False)
 
-    affirmed = Bool(name='affirmed', allow_none=True, default=False,)
+    affirmed = Bool(name='affirmed', allow_none=False, default=False,)
 
     version = Unicode(name='version', allow_none=True, default=None)
 
diff --git a/lib/lp/registry/tests/test_codeofconduct.py b/lib/lp/registry/tests/test_codeofconduct.py
index 049515c..cc61593 100644
--- a/lib/lp/registry/tests/test_codeofconduct.py
+++ b/lib/lp/registry/tests/test_codeofconduct.py
@@ -5,6 +5,7 @@
 
 from textwrap import dedent
 
+from storm.exceptions import NoneError
 from testtools.matchers import (
     ContainsDict,
     Equals,
@@ -16,6 +17,7 @@ from lp.registry.interfaces.codeofconduct import (
     ICodeOfConductSet,
     ISignedCodeOfConductSet,
     )
+from lp.registry.model.codeofconduct import SignedCodeOfConduct
 from lp.services.config import config
 from lp.services.gpg.handler import PymeSignature
 from lp.services.gpg.interfaces import (
@@ -242,3 +244,19 @@ class TestSignedCodeOfConductSet(TestCaseWithFactory):
             u"You have already affirmed the current Code of Conduct.",
             getUtility(ISignedCodeOfConductSet).affirmAndStore(
                 user, current.content))
+
+
+class TestSignedCodeOfConduct(TestCaseWithFactory):
+    layer = ZopelessDatabaseLayer
+
+    def test_affirmed_cant_be_instantiated_with_none(self):
+        self.assertRaises(
+            NoneError,
+            SignedCodeOfConduct,
+            owner=self.factory.makePerson(),
+            affirmed=None
+        )
+
+    def test_affirmed_cant_be_none(self):
+        coc = SignedCodeOfConduct(owner=self.factory.makePerson())
+        self.assertRaises(NoneError, setattr, coc, "affirmed", None)
diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py
index ce28426..e2471dc 100644
--- a/lib/lp/scripts/garbo.py
+++ b/lib/lp/scripts/garbo.py
@@ -75,7 +75,6 @@ from lp.code.model.revision import (
     )
 from lp.oci.model.ocirecipebuild import OCIFile
 from lp.registry.interfaces.person import IPersonSet
-from lp.registry.model.codeofconduct import SignedCodeOfConduct
 from lp.registry.model.person import Person
 from lp.registry.model.product import Product
 from lp.registry.model.sourcepackagename import SourcePackageName
@@ -1765,29 +1764,6 @@ class PopulateSnapBuildStoreRevision(TunableLoop):
         transaction.commit()
 
 
-class PopulateSignedCodeOfConductAffirmed(TunableLoop):
-    """Populates SignedCodeOfConduct.affirmed if not set"""
-
-    maximum_chunk_size = 5000
-
-    def __init__(self, log, abort_time=None):
-        super().__init__(log, abort_time)
-        self.store = IMasterStore(SignedCodeOfConduct)
-
-    def findSignedCodeOfConducts(self):
-        return self.store.find(
-            SignedCodeOfConduct,
-            SignedCodeOfConduct.affirmed == None
-        )
-
-    def isDone(self):
-        return self.findSignedCodeOfConducts().is_empty()
-
-    def __call__(self, chunk_size):
-        self.findSignedCodeOfConducts()[:chunk_size].set(affirmed=False)
-        transaction.commit()
-
-
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -2048,7 +2024,6 @@ class HourlyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
         GitRepositoryPruner,
         RevisionCachePruner,
         UnusedSessionPruner,
-        PopulateSignedCodeOfConductAffirmed,
         ]
     experimental_tunable_loops = []
 
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index c539bf7..a700663 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -84,7 +84,6 @@ from lp.registry.enums import (
 from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.teammembership import TeamMembershipStatus
-from lp.registry.model.codeofconduct import SignedCodeOfConduct
 from lp.registry.model.commercialsubscription import CommercialSubscription
 from lp.registry.model.teammembership import TeamMembership
 from lp.scripts.garbo import (
@@ -97,7 +96,6 @@ from lp.scripts.garbo import (
     load_garbo_job_state,
     LoginTokenPruner,
     OpenIDConsumerAssociationPruner,
-    PopulateSignedCodeOfConductAffirmed,
     PopulateSnapBuildStoreRevision,
     ProductVCSPopulator,
     save_garbo_job_state,
@@ -2047,21 +2045,6 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
         switch_dbuser('testadmin')
         self.assertEqual(build1._store_upload_revision, 1)
 
-    def test_PopulateSignedCodeOfConductAffirmed(self):
-        switch_dbuser('testadmin')
-        populator = PopulateSignedCodeOfConductAffirmed(log=None)
-        for _ in range(5):
-            person = self.factory.makePerson()
-            SignedCodeOfConduct(owner=person).affirmed = None
-
-        result_set = populator.findSignedCodeOfConducts()
-        self.assertGreater(result_set.count(), 0)
-
-        self.runHourly()
-
-        result_set = populator.findSignedCodeOfConducts()
-        self.assertTrue(result_set.is_empty())
-
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer