launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00814
[Merge] lp:~michael.nelson/launchpad/distro-series-difference-model into lp:launchpad
Michael Nelson has proposed merging lp:~michael.nelson/launchpad/distro-series-difference-model into lp:launchpad with lp:~michael.nelson/launchpad/distro-series-difference-message as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#623388 Add model for derived distroseries differences
https://bugs.launchpad.net/bugs/623388
Overview
========
This branch ensures that comments can only be added to a DistroSeriesDifference by people with launchpad.Edit (currently people in the owning team of the derived series).
It also adds a getComments() method, and a method to update the status and/or type of difference based on any new publishings.
This branch follows on from:
https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-message/+merge/33929
Test:
bin/test -vv -m test_distroseriesdifference -m test_distroseriesdifferencecomment
No lint.
--
https://code.launchpad.net/~michael.nelson/launchpad/distro-series-difference-model/+merge/34086
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/distro-series-difference-model into lp:launchpad.
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2010-08-30 16:51:47 +0000
+++ lib/lp/registry/configure.zcml 2010-08-30 16:51:49 +0000
@@ -111,7 +111,10 @@
</securedutility>
<class
class="lp.registry.model.distroseriesdifference.DistroSeriesDifference">
- <allow interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifference"/>
+ <allow interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferencePublic"/>
+ <require
+ permission="launchpad.Edit"
+ interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferenceEdit"/>
</class>
<!-- DistroSeriesDifferenceComment -->
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-08-30 16:51:47 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-08-30 16:51:49 +0000
@@ -8,6 +8,8 @@
__all__ = [
'IDistroSeriesDifference',
+ 'IDistroSeriesDifferencePublic',
+ 'IDistroSeriesDifferenceEdit',
'IDistroSeriesDifferenceSource',
]
@@ -16,7 +18,6 @@
from zope.schema import (
Choice,
Int,
- Text,
TextLine,
)
@@ -26,13 +27,15 @@
DistroSeriesDifferenceType,
)
from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.person import IPerson
from lp.registry.interfaces.sourcepackagename import ISourcePackageName
+from lp.registry.interfaces.role import IHasOwner
from lp.soyuz.interfaces.packagediff import IPackageDiff
from lp.soyuz.interfaces.publishing import ISourcePackagePublishingHistory
-class IDistroSeriesDifference(Interface):
- """An interface for a package difference between two distroseries."""
+class IDistroSeriesDifferencePublic(IHasOwner, Interface):
+ """The public interface for distro series differences."""
id = Int(title=_('ID'), required=True, readonly=True)
@@ -54,10 +57,6 @@
readonly=True, description=_(
"The most recently generated package diff for this difference."))
- activity_log = Text(
- title=_('A log of activity and comments for this difference.'),
- required=False, readonly=True)
-
status = Choice(
title=_('Distro series difference status.'),
description=_('The current status of this difference.'),
@@ -76,20 +75,60 @@
description=_(
"The most recent published version in the derived series."))
+ source_version = TextLine(
+ title=_("Source version"), readonly=True,
+ description=_(
+ "The version of the most recent source publishing in the "
+ "derived series."))
+
parent_source_pub = Reference(
ISourcePackagePublishingHistory,
title=_("Parent source pub"), readonly=True,
description=_(
"The most recent published version in the parent series."))
+ parent_source_version = TextLine(
+ title=_("Parent source version"), readonly=True,
+ description=_(
+ "The version of the most recent source publishing in the "
+ "parent series."))
+
+ owner = Reference(
+ IPerson, title=_("Owning team of the derived series"), readonly=True,
+ description=_(
+ "This attribute mirrors the owner of the derived series."))
+
title = TextLine(
title=_("Title"), readonly=True, required=False, description=_(
"A human-readable name describing this difference."))
+ def updateStatusAndType():
+ """Checks that difference type and status matches current publishings.
+
+ If the record is updated, a relevant comment is added.
+
+ If there is no longer a difference (ie. the versions are
+ the same) then the status is updated to RESOLVED.
+
+ :return: True if the record was updated, False otherwise.
+ """
+
+ def getComments():
+ """Return a result set of the comments for this difference."""
+
+
+class IDistroSeriesDifferenceEdit(Interface):
+ """Difference attributes requiring launchpad.Edit."""
+
def addComment(owner, comment):
"""Add a comment on this difference."""
+class IDistroSeriesDifference(IDistroSeriesDifferencePublic,
+ IDistroSeriesDifferenceEdit):
+ """An interface for a package difference between two distroseries."""
+
+
class IDistroSeriesDifferenceSource(Interface):
"""A utility of this interface can be used to create differences."""
=== modified file 'lib/lp/registry/interfaces/distroseriesdifferencecomment.py'
--- lib/lp/registry/interfaces/distroseriesdifferencecomment.py 2010-08-30 16:51:47 +0000
+++ lib/lp/registry/interfaces/distroseriesdifferencecomment.py 2010-08-30 16:51:49 +0000
@@ -55,3 +55,11 @@
:param comment: The comment.
:return: A new `DistroSeriesDifferenceComment` object.
"""
+
+ def getForDifference(distro_series_difference):
+ """Return a result set of comments for a difference.
+
+ :param distro_series_difference: The distribution series difference
+ for which comments are fetched.
+ :return: A result set of comments, ordered by id by default.
+ """
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-08-30 16:51:47 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-08-30 16:51:49 +0000
@@ -9,7 +9,6 @@
'DistroSeriesDifference',
]
-
from storm.locals import (
Int,
Reference,
@@ -91,23 +90,24 @@
return self._getLatestSourcePub(for_parent=True)
@property
+ def owner(self):
+ """See `IDistroSeriesDifference`."""
+ return self.derived_series.owner
+
+ @property
def title(self):
"""See `IDistroSeriesDifference`."""
parent_name = self.derived_series.parent_series.displayname
return ("Difference between distroseries '%(parent_name)s' and "
"'%(derived_name)s' for package '%(pkg_name)s' "
- "(%(versions)s)" % {
+ "(%(parent_version)s/%(source_version)s)" % {
'parent_name': parent_name,
'derived_name': self.derived_series.displayname,
'pkg_name': self.source_package_name.name,
- 'versions': self._getVersions(),
+ 'parent_version': self.parent_source_version,
+ 'source_version': self.source_version,
})
- @property
- def activity_log(self):
- """See `IDistroSeriesDifference`."""
- return u""
-
def _getLatestSourcePub(self, for_parent=False):
"""Helper to keep source_pub/parent_source_pub DRY."""
distro_series = self.derived_series
@@ -123,16 +123,53 @@
else:
return None
- def _getVersions(self):
- """Helper method returning versions string."""
- src_pub_ver = parent_src_pub_ver = "-"
+ @property
+ def source_version(self):
+ """See `IDistroSeriesDifference`."""
if self.source_pub:
- src_pub_ver = self.source_pub.source_package_version
- if self.parent_source_pub is not None:
- parent_src_pub_ver = self.parent_source_pub.source_package_version
- return parent_src_pub_ver + "/" + src_pub_ver
+ return self.source_pub.source_package_version
+ return ''
+
+ @property
+ def parent_source_version(self):
+ """See `IDistroSeriesDifference`."""
+ if self.parent_source_pub:
+ return self.parent_source_pub.source_package_version
+ return ''
+
+ def updateStatusAndType(self):
+ """See `IDistroSeriesDifference`."""
+ if self.source_pub is None:
+ new_type = DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES
+ elif self.parent_source_pub is None:
+ new_type = DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES
+ else:
+ new_type = DistroSeriesDifferenceType.DIFFERENT_VERSIONS
+
+ updated = False
+ if new_type != self.difference_type:
+ updated = True
+ self.difference_type = new_type
+
+ version = self.source_version
+ parent_version = self.parent_source_version
+ if self.status == DistroSeriesDifferenceStatus.RESOLVED:
+ if version != parent_version:
+ updated = True
+ self.status = DistroSeriesDifferenceStatus.NEEDS_ATTENTION
+ else:
+ if version == parent_version:
+ updated = True
+ self.status = DistroSeriesDifferenceStatus.RESOLVED
+
+ return updated
def addComment(self, owner, comment):
"""See `IDistroSeriesDifference`."""
return getUtility(IDistroSeriesDifferenceCommentSource).new(
self, owner, comment)
+
+ def getComments(self):
+ """See `IDistroSeriesDifference`."""
+ comment_source = getUtility(IDistroSeriesDifferenceCommentSource)
+ return comment_source.getForDifference(self)
=== modified file 'lib/lp/registry/model/distroseriesdifferencecomment.py'
--- lib/lp/registry/model/distroseriesdifferencecomment.py 2010-08-30 16:51:47 +0000
+++ lib/lp/registry/model/distroseriesdifferencecomment.py 2010-08-30 16:51:49 +0000
@@ -65,3 +65,13 @@
dsd_comment.message = message
return store.add(dsd_comment)
+
+ @staticmethod
+ def getForDifference(distro_series_difference):
+ """See `IDistroSeriesDifferenceCommentSource`."""
+ DSDComment = DistroSeriesDifferenceComment
+ from canonical.launchpad.interfaces.lpstorm import IStore
+ comments = IStore(DSDComment).find(
+ DistroSeriesDifferenceComment,
+ DSDComment.distro_series_difference == distro_series_difference)
+ return comments.order_by(DSDComment.id)
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-08-30 16:51:47 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-08-30 16:51:49 +0000
@@ -3,17 +3,27 @@
"""Model tests for the DistroSeriesDifference class."""
+from __future__ import with_statement
+
__metaclass__ = type
import unittest
from storm.store import Store
from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
+from canonical.launchpad.webapp.authorization import check_permission
from canonical.launchpad.webapp.testing import verifyObject
from canonical.testing import DatabaseFunctionalLayer
-from lp.testing import TestCaseWithFactory
-from lp.registry.enum import DistroSeriesDifferenceType
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+from lp.registry.enum import (
+ DistroSeriesDifferenceStatus,
+ DistroSeriesDifferenceType,
+ )
from lp.registry.exceptions import NotADerivedSeriesError
from lp.registry.interfaces.distroseriesdifference import (
IDistroSeriesDifference,
@@ -105,6 +115,116 @@
self.assertEqual(pending_pub, ds_diff.parent_source_pub)
+ def test_source_version(self):
+ # The version of the source in the derived series is returned.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ source_package_name_str="foonew")
+
+ self.assertEqual(
+ ds_diff.source_pub.source_package_version, ds_diff.source_version)
+
+ def test_source_version_none(self):
+ # None is returned for source_version when there is no source pub.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ source_package_name_str="foonew",
+ difference_type=(
+ DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES))
+
+ self.assertEqual('', ds_diff.source_version)
+
+ def test_updateStatusAndType_resolves_difference(self):
+ # Status is set to resolved when versions match.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ source_package_name_str="foonew",
+ versions={
+ 'parent': '1.0',
+ 'derived': '0.9',
+ })
+ new_derived_pub = self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagename=ds_diff.source_package_name,
+ distroseries=ds_diff.derived_series,
+ status=PackagePublishingStatus.PENDING,
+ version='1.0')
+
+ was_updated = ds_diff.updateStatusAndType()
+
+ self.assertIs(True, was_updated)
+ self.assertEqual(
+ DistroSeriesDifferenceStatus.RESOLVED,
+ ds_diff.status)
+
+ def test_updateStatusAndType_re_opens_difference(self):
+ # The status of a resolved difference will updated with new
+ # uploads.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ source_package_name_str="foonew",
+ versions={
+ 'parent': '1.0',
+ 'derived': '1.0',
+ },
+ status=DistroSeriesDifferenceStatus.RESOLVED)
+ new_derived_pub = self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagename=ds_diff.source_package_name,
+ distroseries=ds_diff.derived_series,
+ status=PackagePublishingStatus.PENDING,
+ version='1.1')
+
+ was_updated = ds_diff.updateStatusAndType()
+
+ self.assertIs(True, was_updated)
+ self.assertEqual(
+ DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
+ ds_diff.status)
+
+ def test_updateStatusAndType_new_version_no_change(self):
+ # Uploading a new (different) version does not necessarily
+ # update the record.
+ # In this case, a new version is uploaded, but there is still a
+ # difference needing attention.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ source_package_name_str="foonew",
+ versions={
+ 'parent': '1.0',
+ 'derived': '0.9',
+ })
+ new_derived_pub = self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagename=ds_diff.source_package_name,
+ distroseries=ds_diff.derived_series,
+ status=PackagePublishingStatus.PENDING,
+ version='1.1')
+
+ was_updated = ds_diff.updateStatusAndType()
+
+ self.assertIs(False, was_updated)
+ self.assertEqual(
+ DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
+ ds_diff.status)
+
+ def test_updateStatusAndType_changes_type(self):
+ # The type of difference is updated when appropriate.
+ # In this case, a package that was previously only in the
+ # derived series (UNIQUE_TO_DERIVED_SERIES), is uploaded
+ # to the parent series with a different version.
+ ds_diff = self.factory.makeDistroSeriesDifference(
+ source_package_name_str="foonew",
+ versions={
+ 'derived': '0.9',
+ },
+ difference_type=(
+ DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES))
+ new_parent_pub = self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagename=ds_diff.source_package_name,
+ distroseries=ds_diff.derived_series.parent_series,
+ status=PackagePublishingStatus.PENDING,
+ version='1.1')
+
+ was_updated = ds_diff.updateStatusAndType()
+
+ self.assertIs(True, was_updated)
+ self.assertEqual(
+ DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
+ ds_diff.difference_type)
+
def test_title(self):
# The title is a friendly description of the difference.
parent_series = self.factory.makeDistroSeries(name="lucid")
@@ -126,11 +246,45 @@
# Adding a comment creates a new DistroSeriesDifferenceComment
ds_diff = self.factory.makeDistroSeriesDifference(
source_package_name_str="foonew")
+
+ with person_logged_in(ds_diff.owner):
+ dsd_comment = ds_diff.addComment(
+ ds_diff.owner, "Wait until version 2.1")
+
+ self.assertEqual(ds_diff, dsd_comment.distro_series_difference)
+
+ def test_getComments(self):
+ # All comments for this difference are returned.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ with person_logged_in(ds_diff.owner):
+ dsd_comment = ds_diff.addComment(
+ ds_diff.owner, "Wait until version 2.1")
+ dsd_comment_2 = ds_diff.addComment(
+ ds_diff.owner, "Wait until version 2.1")
+
+ self.assertEqual(
+ [dsd_comment, dsd_comment_2], list(ds_diff.getComments()))
+
+ def test_addComment_not_public(self):
+ # Comments cannot be added with launchpad.View.
+ ds_diff = self.factory.makeDistroSeriesDifference()
person = self.factory.makePerson()
- dsd_comment = ds_diff.addComment(person, "Wait until version 2.1")
-
- self.assertEqual(ds_diff, dsd_comment.distro_series_difference)
+ with person_logged_in(person):
+ self.assertTrue(check_permission('launchpad.View', ds_diff))
+ self.assertFalse(check_permission('launchpad.Edit', ds_diff))
+ self.assertRaises(Unauthorized, getattr, ds_diff, 'addComment')
+
+ def test_addComment_for_owners(self):
+ # Comments can be added by any of the owners of the derived
+ # series.
+ ds_diff = self.factory.makeDistroSeriesDifference()
+
+ with person_logged_in(ds_diff.owner):
+ self.assertTrue(check_permission('launchpad.Edit', ds_diff))
+ diff_comment = ds_diff.addComment(
+ ds_diff.derived_series.owner, "Boo")
def test_suite():
=== modified file 'lib/lp/registry/tests/test_distroseriesdifferencecomment.py'
--- lib/lp/registry/tests/test_distroseriesdifferencecomment.py 2010-08-30 16:51:47 +0000
+++ lib/lp/registry/tests/test_distroseriesdifferencecomment.py 2010-08-30 16:51:49 +0000
@@ -6,11 +6,13 @@
__metaclass__ = type
from storm.store import Store
+from zope.component import getUtility
from canonical.launchpad.webapp.testing import verifyObject
from canonical.testing import DatabaseFunctionalLayer
from lp.registry.interfaces.distroseriesdifferencecomment import (
IDistroSeriesDifferenceComment,
+ IDistroSeriesDifferenceCommentSource,
)
from lp.testing import TestCaseWithFactory
@@ -42,3 +44,21 @@
self.assertEqual(
dsd_comment.distro_series_difference.title,
dsd_comment.message.subject)
+
+
+class DifferenceCommentSourceTestCase(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_getForDifference(self):
+ # Returns all comments for a difference ordered by id.
+ dsd_comment = self.factory.makeDistroSeriesDifferenceComment()
+ dsd_comment_2 = self.factory.makeDistroSeriesDifferenceComment(
+ dsd_comment.distro_series_difference)
+ dsd_other_comment = self.factory.makeDistroSeriesDifferenceComment()
+
+ comment_source = getUtility(IDistroSeriesDifferenceCommentSource)
+ comments = comment_source.getForDifference(
+ distro_series_difference=dsd_comment.distro_series_difference)
+
+ self.assertEqual([dsd_comment, dsd_comment_2], list(comments))
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-08-30 16:51:47 +0000
+++ lib/lp/testing/factory.py 2010-08-30 16:51:49 +0000
@@ -154,7 +154,10 @@
IHWSubmissionDeviceSet,
IHWSubmissionSet,
)
-from lp.registry.enum import DistroSeriesDifferenceType
+from lp.registry.enum import (
+ DistroSeriesDifferenceStatus,
+ DistroSeriesDifferenceType,
+ )
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.distributionmirror import (
MirrorContent,
@@ -1827,7 +1830,8 @@
def makeDistroSeriesDifference(
self, derived_series=None, source_package_name_str=None,
versions=None,
- difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS):
+ difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
+ status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
"""Create a new distro series source package difference."""
if derived_series is None:
parent_series = self.makeDistroSeries()
@@ -1860,7 +1864,8 @@
sourcepackagename=source_package_name)
return getUtility(IDistroSeriesDifferenceSource).new(
- derived_series, source_package_name, difference_type)
+ derived_series, source_package_name, difference_type,
+ status=status)
def makeDistroSeriesDifferenceComment(
self, distro_series_difference=None, owner=None, comment=None):
Follow ups
