launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04165
[Merge] lp:~julian-edwards/launchpad/set-previous-series-bug-805913 into lp:launchpad
Julian Edwards has proposed merging lp:~julian-edwards/launchpad/set-previous-series-bug-805913 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #805913 in Launchpad itself: "previous_series (parent_series in DB) is no longer set when opening a new distroseries"
https://bugs.launchpad.net/launchpad/+bug/805913
For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/set-previous-series-bug-805913/+merge/66942
= Summary =
Make DistroSeries.previous_series get auto-populated when a new series is created.
== Implementation details ==
1. rename prior_series to priorReleasedSeries and convert to a method (only one place was using it and it didn't need to be a cachedproperty)
2. refactor the code in priorReleasedSeries so it's also in IDistroSeriesSet.
3. Stormify the query.
4. Add tests for it! (there were none before)
5. Make the browser code for adding a new series call the utility method to work out what the previous_series should be.
6. Tests for that.
== Tests ==
bin/test -cvvt TestDistroSeriesAddView -t test_priorReleasedSeries
== Demo and Q/A ==
+initseries currently blows up when initialising because of the missing data,
this will fix it and easily checked.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/tests/test_distroseries.py
lib/lp/registry/interfaces/distroseries.py
lib/lp/registry/browser/tests/test_distroseries.py
lib/lp/registry/model/sourcepackage.py
lib/lp/registry/model/distroseries.py
lib/lp/registry/browser/distroseries.py
--
https://code.launchpad.net/~julian-edwards/launchpad/set-previous-series-bug-805913/+merge/66942
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/set-previous-series-bug-805913 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-06-28 17:57:34 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-07-05 17:49:25 +0000
@@ -93,7 +93,10 @@
DistroSeriesDifferenceStatus,
DistroSeriesDifferenceType,
)
-from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.distroseries import (
+ IDistroSeries,
+ IDistroSeriesSet,
+ )
from lp.registry.interfaces.distroseriesdifference import (
IDistroSeriesDifferenceSource,
)
@@ -620,6 +623,10 @@
@action(_('Add Series'), name='create')
def createAndAdd(self, action, data):
"""Create and add a new Distribution Series"""
+ all_previous_series = getUtility(
+ IDistroSeriesSet).priorReleasedSeries(self.context, UTC_NOW)
+ previous_series = all_previous_series.first()
+ # previous_series will be None if there isn't one.
distroseries = self.context.newSeries(
name=data['name'],
displayname=data['displayname'],
@@ -627,7 +634,7 @@
summary=data['summary'],
description=u"",
version=data['version'],
- previous_series=None,
+ previous_series=previous_series,
registrant=self.user)
notify(ObjectCreatedEvent(distroseries))
self.next_url = canonical_url(distroseries)
=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py 2011-06-28 17:57:34 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-07-05 17:49:25 +0000
@@ -5,6 +5,7 @@
__metaclass__ = type
+from datetime import timedelta
import difflib
import re
from textwrap import TextWrapper
@@ -62,6 +63,7 @@
getFeatureFlag,
)
from lp.services.features.testing import FeatureFixture
+from lp.services.utils import utc_now
from lp.soyuz.enums import (
ArchivePermissionType,
PackagePublishingStatus,
@@ -469,13 +471,12 @@
layer = DatabaseFunctionalLayer
- def test_submit(self):
- # When creating a new DistroSeries via DistroSeriesAddView, the title
- # is set to the same as the displayname (title is, in any case,
- # deprecated), the description is left empty, and previous_series is
- # None (DistroSeriesInitializeView takes care of setting that).
- user = self.factory.makePerson()
- distribution = self.factory.makeDistribution(owner=user)
+ def setUp(self):
+ super(TestDistroSeriesAddView, self).setUp()
+ self.user = self.factory.makePerson()
+ self.distribution = self.factory.makeDistribution(owner=self.user)
+
+ def createNewDistroseries(self):
form = {
"field.name": u"polished",
"field.version": u"12.04",
@@ -483,17 +484,38 @@
"field.summary": u"Even The Register likes it.",
"field.actions.create": u"Add Series",
}
- with person_logged_in(user):
- create_initialized_view(distribution, "+addseries", form=form)
- distroseries = distribution.getSeries(u"polished")
+ with person_logged_in(self.user):
+ create_initialized_view(self.distribution, "+addseries",
+ form=form)
+ distroseries = self.distribution.getSeries(u"polished")
+ return distroseries
+
+ def assertCreated(self, distroseries):
self.assertEqual(u"polished", distroseries.name)
self.assertEqual(u"12.04", distroseries.version)
self.assertEqual(u"Polished Polecat", distroseries.displayname)
self.assertEqual(u"Polished Polecat", distroseries.title)
self.assertEqual(u"Even The Register likes it.", distroseries.summary)
self.assertEqual(u"", distroseries.description)
+ self.assertEqual(self.user, distroseries.owner)
+
+ def test_plain_submit(self):
+ # When creating a new DistroSeries via DistroSeriesAddView, the title
+ # is set to the same as the displayname (title is, in any case,
+ # deprecated), the description is left empty, and previous_series is
+ # None (DistroSeriesInitializeView takes care of setting that).
+ distroseries = self.createNewDistroseries()
+ self.assertCreated(distroseries)
self.assertIs(None, distroseries.previous_series)
- self.assertEqual(user, distroseries.owner)
+
+ def test_submit_sets_previous_series(self):
+ # Creating a new series when one already exists should set the
+ # previous_series.
+ old_series = self.factory.makeDistroSeries(self.distribution)
+ old_time = utc_now() - timedelta(days=5)
+ removeSecurityProxy(old_series).datereleased = old_time
+ distroseries = self.createNewDistroseries()
+ self.assertEqual(old_series, distroseries.previous_series)
class TestDistroSeriesInitializeView(TestCaseWithFactory):
=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py 2011-06-20 16:04:15 +0000
+++ lib/lp/registry/interfaces/distroseries.py 2011-07-05 17:49:25 +0000
@@ -360,9 +360,8 @@
automatically upgrade within backports, but not into it.
"""))
- # other properties
- prior_series = Attribute(
- "Prior series *by date* from the same distribution.")
+ def priorReleasedSeries():
+ """Prior series *by date* from the same distribution."""
main_archive = exported(
Reference(
@@ -1046,6 +1045,18 @@
released == None will do no filtering on status.
"""
+ def priorReleasedSeries(self, distribution, prior_to_date):
+ """Find distroseries for the supplied distro released before a
+ certain date.
+
+ :param distribution: An `IDistribution` in which to search for its
+ series.
+ :param prior_to_date: A `datetime`
+
+ :return: `IResultSet` of `IDistroSeries` that were released before
+ prior_to_date, ordered in increasing order of age.
+ """
+
@error_status(httplib.BAD_REQUEST)
class DerivationError(Exception):
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py 2011-06-17 14:23:52 +0000
+++ lib/lp/registry/model/distroseries.py 2011-07-05 17:49:25 +0000
@@ -685,22 +685,14 @@
orderBy=["Language.englishname"])
return result
- @cachedproperty
- def prior_series(self):
+ def priorReleasedSeries(self):
"""See `IDistroSeries`."""
- # This property is cached because it is used intensely inside
- # sourcepackage.py; avoiding regeneration reduces a lot of
- # count(*) queries.
datereleased = self.datereleased
# if this one is unreleased, use the last released one
if not datereleased:
- datereleased = 'NOW'
- results = DistroSeries.select('''
- distribution = %s AND
- datereleased < %s
- ''' % sqlvalues(self.distribution.id, datereleased),
- orderBy=['-datereleased'])
- return list(results)
+ datereleased = UTC_NOW
+ return getUtility(IDistroSeriesSet).priorReleasedSeries(
+ self.distribution, datereleased)
@property
def bug_reporting_guidelines(self):
@@ -2021,4 +2013,16 @@
if orderBy is not None:
return DistroSeries.select(where_clause, orderBy=orderBy)
else:
+
return DistroSeries.select(where_clause)
+
+ def priorReleasedSeries(self, distribution, prior_to_date):
+ """See `IDistroSeriesSet`."""
+ store = Store.of(distribution)
+ results = store.find(
+ DistroSeries,
+ DistroSeries.distributionID == distribution.id,
+ DistroSeries.datereleased < prior_to_date
+ ).order_by(Desc(DistroSeries.datereleased))
+
+ return results
=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py 2011-06-08 15:00:44 +0000
+++ lib/lp/registry/model/sourcepackage.py 2011-07-05 17:49:25 +0000
@@ -44,7 +44,6 @@
BugTargetBase,
HasBugHeatMixin,
)
-from lp.bugs.model.bugtask import BugTask
from lp.buildmaster.enums import BuildStatus
from lp.code.model.seriessourcepackagebranch import (
SeriesSourcePackageBranchSet,
@@ -428,7 +427,7 @@
# if we are an ubuntu sourcepackage, try the previous series of
# ubuntu
if self.distribution == ubuntu:
- ubuntuseries = self.distroseries.prior_series
+ ubuntuseries = self.distroseries.priorReleasedSeries()
if ubuntuseries:
previous_ubuntu_series = ubuntuseries[0]
sp = SourcePackage(sourcepackagename=self.sourcepackagename,
@@ -500,7 +499,8 @@
"""See `IBugTarget`."""
return self.distroseries.getUsedBugTags()
- def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
+ def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0,
+ include_tags=None):
"""See IBugTarget."""
# Circular fail.
from lp.bugs.model.bugsummary import BugSummary
=== modified file 'lib/lp/registry/tests/test_distroseries.py'
--- lib/lp/registry/tests/test_distroseries.py 2011-06-14 19:49:18 +0000
+++ lib/lp/registry/tests/test_distroseries.py 2011-07-05 17:49:25 +0000
@@ -5,6 +5,9 @@
__metaclass__ = type
+from datetime import (
+ timedelta,
+)
import transaction
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -20,6 +23,7 @@
from lp.registry.errors import NoSuchDistroSeries
from lp.registry.interfaces.distroseries import IDistroSeriesSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.utils import utc_now
from lp.soyuz.enums import (
ArchivePurpose,
PackagePublishingStatus,
@@ -256,6 +260,29 @@
distroseries=distroseries, archive=distroseries.main_archive)
self.assertTrue(distroseries.isInitialized())
+ def test_priorReleasedSeries(self):
+ # Make sure that previousReleasedSeries returns all series with a
+ # release date less than the contextual series,
+ # ordered by descending date.
+ distro = self.factory.makeDistribution()
+ ds1 = self.factory.makeDistroSeries(distribution=distro)
+ ds2 = self.factory.makeDistroSeries(distribution=distro)
+ ds3 = self.factory.makeDistroSeries(distribution=distro)
+ ds4 = self.factory.makeDistroSeries(distribution=distro)
+
+ now = utc_now()
+ older = now - timedelta(days=5)
+ oldest = now - timedelta(days=10)
+ newer = now + timedelta(days=15)
+ removeSecurityProxy(ds1).datereleased = oldest
+ removeSecurityProxy(ds2).datereleased = older
+ removeSecurityProxy(ds4).datereleased = newer
+
+ prior = ds3.priorReleasedSeries()
+ self.assertEqual(
+ [ds2, ds1],
+ list(prior))
+
class TestDistroSeriesPackaging(TestCaseWithFactory):
