Hi Edwin, This change looks great. I marked a couple of niggly bits but that's all. --bac > === added file 'database/schema/patch-2207-56-0.sql' > --- database/schema/patch-2207-56-0.sql 1970-01-01 00:00:00 +0000 > +++ database/schema/patch-2207-56-0.sql 2010-06-04 18:48:29 +0000 > @@ -0,0 +1,24 @@ > +-- Copyright 2009 Canonical Ltd. This software is licensed under the 2010 (The contents of the patch are left for a db review.) > === modified file 'lib/lp/bugs/tests/test_bugheat.py' > --- lib/lp/bugs/tests/test_bugheat.py 2010-06-01 09:21:32 +0000 > +++ lib/lp/bugs/tests/test_bugheat.py 2010-06-04 18:48:29 +0000 > @@ -7,8 +7,11 @@ Edwin this test is really readable and easy to follow. Good job. > import unittest > > +from storm.store import Store > + > from canonical.testing import LaunchpadZopelessLayer > > +from lp.testing import TestCaseWithFactory > from lp.testing.factory import LaunchpadObjectFactory > > > @@ -58,6 +61,85 @@ > def setUp(self): > self.target = self.factory.makeDistributionSourcePackage() > > +class DistributionSourcePackageNullBugHeatCacheTest( > + TestCaseWithFactory): > + """Ensure distro source package cache values start at None.""" > + > + layer = LaunchpadZopelessLayer > + > + def setUp(self): > + TestCaseWithFactory.setUp(self) > + self.target = self.factory.makeDistributionSourcePackage() > + > + def test_null_max_bug_heat(self): > + self.assertEqual(None, self.target.max_bug_heat) > + > + def test_null_total_bug_heat(self): > + self.assertEqual(None, self.target.total_bug_heat) > + > + def test_null_bug_count(self): > + self.assertEqual(None, self.target.bug_count) > + > + > +class DistributionSourcePackageZeroRecalculateBugHeatCacheTest( > + TestCaseWithFactory): > + """Ensure distro source package cache values become zero properly.""" > + > + layer = LaunchpadZopelessLayer > + > + def setUp(self): > + TestCaseWithFactory.setUp(self) > + self.target = self.factory.makeDistributionSourcePackage() > + self.target.recalculateBugHeatCache() > + > + def test_zero_max_bug_heat(self): > + self.assertEqual(0, self.target.max_bug_heat) > + > + def test_zero_total_bug_heat(self): > + self.assertEqual(0, self.target.total_bug_heat) > + > + def test_zero_bug_count(self): > + self.assertEqual(0, self.target.bug_count) > + > + > +class DistributionSourcePackageMultipleBugsRecalculateBugHeatCacheTest( > + TestCaseWithFactory): > + """Ensure distro source package cache values are set properly.""" > + > + layer = LaunchpadZopelessLayer > + > + def setUp(self): > + TestCaseWithFactory.setUp(self) > + self.target = self.factory.makeDistributionSourcePackage() > + self.bugtask1 = self.factory.makeBugTask(target=self.target) > + self.bugtask2 = self.factory.makeBugTask(target=self.target) > + # Bug heat gets calculated by complicated rules in a db > + # stored procedure. We will override them here to avoid > + # testing inconsitencies if those values are calculated > + # differently in the future. > + # target.recalculateBugHeatCache() should be called > + # automatically by bug.setHeat(). > + bug1 = self.bugtask1.bug > + bug2 = self.bugtask2.bug > + bug1.setHeat(7) > + bug2.setHeat(19) > + Store.of(bug1).flush() > + self.max_heat = max(bug1.heat, bug2.heat) > + self.total_heat = sum([bug1.heat, bug2.heat]) > + > + def test_max_bug_heat(self): > + self.assertEqual(self.max_heat, self.target.max_bug_heat) > + > + def test_total_bug_heat(self): > + self.assertEqual(self.total_heat, self.target.total_bug_heat) > + self.failUnless( > + self.target.total_bug_heat > self.target.max_bug_heat, > + "Total bug heat should be more than the max bug heat, " > + "since we know that multiple bugs have nonzero heat.") > + > + def test_bug_count(self): > + self.assertEqual(2, self.target.bug_count) > + > > class SourcePackageMaxHeatByTargetTest( > MaxHeatByTargetBase, unittest.TestCase): > === modified file 'lib/lp/registry/interfaces/distributionsourcepackage.py' > --- lib/lp/registry/interfaces/distributionsourcepackage.py 2010-02-12 13:39:56 +0000 > +++ lib/lp/registry/interfaces/distributionsourcepackage.py 2010-06-04 18:48:29 +0000 > @@ -78,6 +78,22 @@ > "no such package -- this occurs when there is no current series for " > "the distribution.") > > + total_bug_heat = Attribute( > + "Sum of the bug heat for all the bugs matching the distribution " > + "and sourcepackagename of the IDistributionSourcePackage.") > + > + max_bug_heat = Attribute( > + "Maximum bug heat for a single bug matching the distribution " > + "and sourcepackagename of the IDistributionSourcePackage.") > + > + bug_count = Attribute( > + "Number of bugs matching the distribution and sourcepackagename " > + "of the IDistributionSourcePackage.") > + > + po_message_count = Attribute( > + "Number of translations matching the distribution and " > + "sourcepackagename of the IDistributionSourcePackage.") > + > def getReleasesAndPublishingHistory(): > """Return a list of all releases of this source package in this > distribution and their correspodning publishing history. Typo: corresponding (not yours) > === modified file 'lib/lp/registry/model/distributionsourcepackage.py' > --- lib/lp/registry/model/distributionsourcepackage.py 2010-03-19 11:13:00 +0000 > +++ lib/lp/registry/model/distributionsourcepackage.py 2010-06-04 18:48:29 +0000 > @@ -54,6 +59,40 @@ > CustomLanguageCode, HasCustomLanguageCodesMixin) > > > +class DistributionSourcePackageProperty: > + def __init__(self, attrname): > + self.attrname = attrname > + > + def __get__(self, obj, class_): > + return getattr(obj._self_in_database, self.attrname, None) > + > + def __set__(self, obj, value): > + if obj._self_in_database is None: > + # Log an oops without raising an error. > + exception = AssertionError( > + "DistributionSourcePackage record should have been created " > + "earlier in the database for distro=%s, sourcepackagename=%s" > + % (obj.distribution.name, obj.sourcepackagename.name)) Will this generate an OOPS report? > + getUtility(IErrorReportingUtility).raising( > + (exception.__class__, exception, None)) > + spph = Store.of(obj.distribution).find( > + SourcePackagePublishingHistory, > + SourcePackagePublishingHistory.distroseriesID == > + DistroSeries.id, > + DistroSeries.distributionID == obj.distribution.id, > + SourcePackagePublishingHistory.sourcepackagereleaseID == > + SourcePackageRelease.id, > + SourcePackageRelease.sourcepackagenameID == > + obj.sourcepackagename.id > + ).order_by(Desc(SourcePackagePublishingHistory.id)).first() > + if spph is None: > + section = getUtility(ISectionSet)['misc'] > + else: > + section = spph.section > + obj._new(obj.distribution, obj.sourcepackagename, section) > + setattr(obj._self_in_database, self.attrname, value) > + > + > class DistributionSourcePackage(BugTargetBase, > SourcePackageQuestionTargetMixin, > StructuralSubscriptionTargetMixin, > @@ -72,6 +111,14 @@ > IDistributionSourcePackage, IHasBugHeat, IHasCustomLanguageCodes, > IQuestionTarget) > > + bug_reporting_guidelines = DistributionSourcePackageProperty( > + 'bug_reporting_guidelines') > + max_bug_heat = DistributionSourcePackageProperty('max_bug_heat') > + total_bug_heat = DistributionSourcePackageProperty('total_bug_heat') > + bug_count = DistributionSourcePackageProperty('bug_count') > + po_message_count = DistributionSourcePackageProperty('po_message_count') > + section = DistributionSourcePackageProperty('section') > + > def __init__(self, distribution, sourcepackagename): > self.distribution = distribution > self.sourcepackagename = sourcepackagename