Launchpad itself

Merge lp:~michael.nelson/launchpad/distro-series-difference-model into lp:launchpad/db-devel

  1. distro-series-difference-model
  2. Merge into db-devel
Proposed by Michael Nelson
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: 9729
Proposed branch: lp:~michael.nelson/launchpad/distro-series-difference-model
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~michael.nelson/launchpad/distro-series-difference-message
Diff against target: 509 lines (+280/-32)
6 files modified
lib/lp/registry/configure.zcml (+4/-1)
lib/lp/registry/interfaces/distroseriesdifference.py (+46/-7)
lib/lp/registry/model/distroseriesdifference.py (+61/-16)
lib/lp/registry/tests/test_distroseriesdifference.py (+159/-5)
lib/lp/registry/tests/test_distroseriesdifferencecomment.py (+2/-0)
lib/lp/testing/factory.py (+8/-3)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/distro-series-difference-model
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+34086@code.launchpad.net

Commit message

Restrict DistroSeriesDifference.addComment() to lp.Edit and add DSD.updateStatusAndType().

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

Why are source_version and parent_source_version returning ''? I would have expected them to return None if there was no such value.

Why not implement getComments directly instead of indirecting through IDistroSeriesDifferenceCommentSource?

Your test comments say None, but test for the empty string. Please fix either the test or the comment.

Why assertIs(True, was_updated) instead of assertTrue(was_updated)? We should only care about whether the returned value evaluates to True. We should definitely not care about its identity.

Why assertIs(True, was_updated) instead of assertTrue(ds_diff.updateStatusAndType())? It is shorter, so easier to read.

Revision history for this message
Michael Nelson (michael.nelson) wrote :

On Mon, Aug 30, 2010 at 7:18 PM, Aaron Bentley <email address hidden> wrote:
> Why are source_version and parent_source_version returning ''?  I would have expected them to return None if there was no such value.

They were originally returning None (as you saw by the test comment
below), but I'd thought for comparisons etc., it would be more
consistent if these properties always resulted in a string - an empty
one if there is no version. Anyway, I've switched them back to return
None.

>
> Why not implement getComments directly instead of indirecting through IDistroSeriesDifferenceCommentSource?

Because there's no reason why DistroSeriesDifference needs to know
about the implementation of DistroSeriesDifferenceComment ("Program to
interfaces, not implementations", "Depend on abstractions, do not
depend on concrete classes", etc.)

Currently DistroSeriesDifference doesn't import any model code, nor
does it do any storm queries directly. I thought that was a good
thing, but can change it if you want.

>
> Your test comments say None, but test for the empty string.  Please fix either the test or the comment.

Tests updated to expect None.

>
> Why assertIs(True, was_updated) instead of assertTrue(was_updated)?  We should only care about whether the returned value evaluates to True.  We should definitely not care about its identity.

My bad.

>
> Why assertIs(True, was_updated) instead of assertTrue(ds_diff.updateStatusAndType())?  It is shorter, so easier to read.

Ditto.

Thanks Aaron.

changes_aaron.diff
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-08-30 15:09:17 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-08-31 07:16:47 +0000
@@ -128,14 +128,14 @@
128 """See `IDistroSeriesDifference`."""128 """See `IDistroSeriesDifference`."""
129 if self.source_pub:129 if self.source_pub:
130 return self.source_pub.source_package_version130 return self.source_pub.source_package_version
131 return ''131 return None
132132
133 @property133 @property
134 def parent_source_version(self):134 def parent_source_version(self):
135 """See `IDistroSeriesDifference`."""135 """See `IDistroSeriesDifference`."""
136 if self.parent_source_pub:136 if self.parent_source_pub:
137 return self.parent_source_pub.source_package_version137 return self.parent_source_pub.source_package_version
138 return ''138 return None
139139
140 def updateStatusAndType(self):140 def updateStatusAndType(self):
141 """See `IDistroSeriesDifference`."""141 """See `IDistroSeriesDifference`."""
142142
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-08-30 15:09:17 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-08-31 07:36:32 +0000
@@ -130,7 +130,7 @@
130 difference_type=(130 difference_type=(
131 DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES))131 DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES))
132132
133 self.assertEqual('', ds_diff.source_version)133 self.assertEqual(None, ds_diff.source_version)
134134
135 def test_updateStatusAndType_resolves_difference(self):135 def test_updateStatusAndType_resolves_difference(self):
136 # Status is set to resolved when versions match.136 # Status is set to resolved when versions match.
@@ -148,7 +148,7 @@
148148
149 was_updated = ds_diff.updateStatusAndType()149 was_updated = ds_diff.updateStatusAndType()
150150
151 self.assertIs(True, was_updated)151 self.assertTrue(was_updated)
152 self.assertEqual(152 self.assertEqual(
153 DistroSeriesDifferenceStatus.RESOLVED,153 DistroSeriesDifferenceStatus.RESOLVED,
154 ds_diff.status)154 ds_diff.status)
@@ -171,7 +171,7 @@
171171
172 was_updated = ds_diff.updateStatusAndType()172 was_updated = ds_diff.updateStatusAndType()
173173
174 self.assertIs(True, was_updated)174 self.assertTrue(was_updated)
175 self.assertEqual(175 self.assertEqual(
176 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,176 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
177 ds_diff.status)177 ds_diff.status)
@@ -195,7 +195,7 @@
195195
196 was_updated = ds_diff.updateStatusAndType()196 was_updated = ds_diff.updateStatusAndType()
197197
198 self.assertIs(False, was_updated)198 self.assertFalse(was_updated)
199 self.assertEqual(199 self.assertEqual(
200 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,200 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
201 ds_diff.status)201 ds_diff.status)
@@ -220,7 +220,7 @@
220220
221 was_updated = ds_diff.updateStatusAndType()221 was_updated = ds_diff.updateStatusAndType()
222222
223 self.assertIs(True, was_updated)223 self.assertTrue(was_updated)
224 self.assertEqual(224 self.assertEqual(
225 DistroSeriesDifferenceType.DIFFERENT_VERSIONS,225 DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
226 ds_diff.difference_type)226 ds_diff.difference_type)
Revision history for this message
Robert Collins (lifeless) wrote :

@Michael regarding interfaces and model code; if our implementations
were substitutable *with* high performance, I'd totally support what
you're saying.

However, the reality is that object traversal is slow (because of lazy
loading); our model objects conflate database access with domain
knowledge, and these two things combined mean that:
 - you cannot replace Person in calls wanting IPerson with something
derived from Person and stored in another table: the whole structure
of model->storm relationships means that that will simply not work.
 - working solely via I* guarantees terrible performance and the
inability to drive the lower layers efficiently.

I would like to change this, as has been discussed on the list, but we
don't have a better pattern || code for it yet.

You should, of course, delegate, DRY and so forth, but Interface vs
Implementation as far as Launchpad Interfaces go is a weak story.

-Rob

Revision history for this message
Aaron Bentley (abentley) wrote :

Approved, assuming the changes in http://pastebin.ubuntu.com/486363/

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2010-08-31 15:59:47 +0000
+++ lib/lp/registry/configure.zcml 2010-08-31 15:59:49 +0000
@@ -111,7 +111,10 @@
111 </securedutility>111 </securedutility>
112 <class112 <class
113 class="lp.registry.model.distroseriesdifference.DistroSeriesDifference">113 class="lp.registry.model.distroseriesdifference.DistroSeriesDifference">
114 <allow interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifference"/>114 <allow interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferencePublic"/>
115 <require
116 permission="launchpad.Edit"
117 interface="lp.registry.interfaces.distroseriesdifference.IDistroSeriesDifferenceEdit"/>
115 </class>118 </class>
116119
117 <!-- DistroSeriesDifferenceComment -->120 <!-- DistroSeriesDifferenceComment -->
118121
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-08-31 15:59:47 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-08-31 15:59:49 +0000
@@ -8,6 +8,8 @@
88
9__all__ = [9__all__ = [
10 'IDistroSeriesDifference',10 'IDistroSeriesDifference',
11 'IDistroSeriesDifferencePublic',
12 'IDistroSeriesDifferenceEdit',
11 'IDistroSeriesDifferenceSource',13 'IDistroSeriesDifferenceSource',
12 ]14 ]
1315
@@ -16,7 +18,6 @@
16from zope.schema import (18from zope.schema import (
17 Choice,19 Choice,
18 Int,20 Int,
19 Text,
20 TextLine,21 TextLine,
21 )22 )
2223
@@ -26,13 +27,15 @@
26 DistroSeriesDifferenceType,27 DistroSeriesDifferenceType,
27 )28 )
28from lp.registry.interfaces.distroseries import IDistroSeries29from lp.registry.interfaces.distroseries import IDistroSeries
30from lp.registry.interfaces.person import IPerson
29from lp.registry.interfaces.sourcepackagename import ISourcePackageName31from lp.registry.interfaces.sourcepackagename import ISourcePackageName
32from lp.registry.interfaces.role import IHasOwner
30from lp.soyuz.interfaces.packagediff import IPackageDiff33from lp.soyuz.interfaces.packagediff import IPackageDiff
31from lp.soyuz.interfaces.publishing import ISourcePackagePublishingHistory34from lp.soyuz.interfaces.publishing import ISourcePackagePublishingHistory
3235
3336
34class IDistroSeriesDifference(Interface):37class IDistroSeriesDifferencePublic(IHasOwner, Interface):
35 """An interface for a package difference between two distroseries."""38 """The public interface for distro series differences."""
3639
37 id = Int(title=_('ID'), required=True, readonly=True)40 id = Int(title=_('ID'), required=True, readonly=True)
3841
@@ -54,10 +57,6 @@
54 readonly=True, description=_(57 readonly=True, description=_(
55 "The most recently generated package diff for this difference."))58 "The most recently generated package diff for this difference."))
5659
57 activity_log = Text(
58 title=_('A log of activity and comments for this difference.'),
59 required=False, readonly=True)
60
61 status = Choice(60 status = Choice(
62 title=_('Distro series difference status.'),61 title=_('Distro series difference status.'),
63 description=_('The current status of this difference.'),62 description=_('The current status of this difference.'),
@@ -76,20 +75,60 @@
76 description=_(75 description=_(
77 "The most recent published version in the derived series."))76 "The most recent published version in the derived series."))
7877
78 source_version = TextLine(
79 title=_("Source version"), readonly=True,
80 description=_(
81 "The version of the most recent source publishing in the "
82 "derived series."))
83
79 parent_source_pub = Reference(84 parent_source_pub = Reference(
80 ISourcePackagePublishingHistory,85 ISourcePackagePublishingHistory,
81 title=_("Parent source pub"), readonly=True,86 title=_("Parent source pub"), readonly=True,
82 description=_(87 description=_(
83 "The most recent published version in the parent series."))88 "The most recent published version in the parent series."))
8489
90 parent_source_version = TextLine(
91 title=_("Parent source version"), readonly=True,
92 description=_(
93 "The version of the most recent source publishing in the "
94 "parent series."))
95
96 owner = Reference(
97 IPerson, title=_("Owning team of the derived series"), readonly=True,
98 description=_(
99 "This attribute mirrors the owner of the derived series."))
100
85 title = TextLine(101 title = TextLine(
86 title=_("Title"), readonly=True, required=False, description=_(102 title=_("Title"), readonly=True, required=False, description=_(
87 "A human-readable name describing this difference."))103 "A human-readable name describing this difference."))
88104
105 def updateStatusAndType():
106 """Checks that difference type and status matches current publishings.
107
108 If the record is updated, a relevant comment is added.
109
110 If there is no longer a difference (ie. the versions are
111 the same) then the status is updated to RESOLVED.
112
113 :return: True if the record was updated, False otherwise.
114 """
115
116 def getComments():
117 """Return a result set of the comments for this difference."""
118
119
120class IDistroSeriesDifferenceEdit(Interface):
121 """Difference attributes requiring launchpad.Edit."""
122
89 def addComment(owner, comment):123 def addComment(owner, comment):
90 """Add a comment on this difference."""124 """Add a comment on this difference."""
91125
92126
127class IDistroSeriesDifference(IDistroSeriesDifferencePublic,
128 IDistroSeriesDifferenceEdit):
129 """An interface for a package difference between two distroseries."""
130
131
93class IDistroSeriesDifferenceSource(Interface):132class IDistroSeriesDifferenceSource(Interface):
94 """A utility of this interface can be used to create differences."""133 """A utility of this interface can be used to create differences."""
95134
96135
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-08-31 15:59:47 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-08-31 15:59:49 +0000
@@ -9,7 +9,6 @@
9 'DistroSeriesDifference',9 'DistroSeriesDifference',
10 ]10 ]
1111
12
13from storm.locals import (12from storm.locals import (
14 Int,13 Int,
15 Reference,14 Reference,
@@ -22,7 +21,10 @@
22 )21 )
2322
24from canonical.database.enumcol import DBEnum23from canonical.database.enumcol import DBEnum
25from canonical.launchpad.interfaces.lpstorm import IMasterStore24from canonical.launchpad.interfaces.lpstorm import (
25 IMasterStore,
26 IStore,
27 )
26from lp.registry.enum import (28from lp.registry.enum import (
27 DistroSeriesDifferenceStatus,29 DistroSeriesDifferenceStatus,
28 DistroSeriesDifferenceType,30 DistroSeriesDifferenceType,
@@ -35,6 +37,8 @@
35from lp.registry.interfaces.distroseriesdifferencecomment import (37from lp.registry.interfaces.distroseriesdifferencecomment import (
36 IDistroSeriesDifferenceCommentSource,38 IDistroSeriesDifferenceCommentSource,
37 )39 )
40from lp.registry.model.distroseriesdifferencecomment import (
41 DistroSeriesDifferenceComment)
3842
3943
40class DistroSeriesDifference(Storm):44class DistroSeriesDifference(Storm):
@@ -91,23 +95,24 @@
91 return self._getLatestSourcePub(for_parent=True)95 return self._getLatestSourcePub(for_parent=True)
9296
93 @property97 @property
98 def owner(self):
99 """See `IDistroSeriesDifference`."""
100 return self.derived_series.owner
101
102 @property
94 def title(self):103 def title(self):
95 """See `IDistroSeriesDifference`."""104 """See `IDistroSeriesDifference`."""
96 parent_name = self.derived_series.parent_series.displayname105 parent_name = self.derived_series.parent_series.displayname
97 return ("Difference between distroseries '%(parent_name)s' and "106 return ("Difference between distroseries '%(parent_name)s' and "
98 "'%(derived_name)s' for package '%(pkg_name)s' "107 "'%(derived_name)s' for package '%(pkg_name)s' "
99 "(%(versions)s)" % {108 "(%(parent_version)s/%(source_version)s)" % {
100 'parent_name': parent_name,109 'parent_name': parent_name,
101 'derived_name': self.derived_series.displayname,110 'derived_name': self.derived_series.displayname,
102 'pkg_name': self.source_package_name.name,111 'pkg_name': self.source_package_name.name,
103 'versions': self._getVersions(),112 'parent_version': self.parent_source_version,
113 'source_version': self.source_version,
104 })114 })
105115
106 @property
107 def activity_log(self):
108 """See `IDistroSeriesDifference`."""
109 return u""
110
111 def _getLatestSourcePub(self, for_parent=False):116 def _getLatestSourcePub(self, for_parent=False):
112 """Helper to keep source_pub/parent_source_pub DRY."""117 """Helper to keep source_pub/parent_source_pub DRY."""
113 distro_series = self.derived_series118 distro_series = self.derived_series
@@ -123,16 +128,56 @@
123 else:128 else:
124 return None129 return None
125130
126 def _getVersions(self):131 @property
127 """Helper method returning versions string."""132 def source_version(self):
128 src_pub_ver = parent_src_pub_ver = "-"133 """See `IDistroSeriesDifference`."""
129 if self.source_pub:134 if self.source_pub:
130 src_pub_ver = self.source_pub.source_package_version135 return self.source_pub.source_package_version
131 if self.parent_source_pub is not None:136 return None
132 parent_src_pub_ver = self.parent_source_pub.source_package_version137
133 return parent_src_pub_ver + "/" + src_pub_ver138 @property
139 def parent_source_version(self):
140 """See `IDistroSeriesDifference`."""
141 if self.parent_source_pub:
142 return self.parent_source_pub.source_package_version
143 return None
144
145 def updateStatusAndType(self):
146 """See `IDistroSeriesDifference`."""
147 if self.source_pub is None:
148 new_type = DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES
149 elif self.parent_source_pub is None:
150 new_type = DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES
151 else:
152 new_type = DistroSeriesDifferenceType.DIFFERENT_VERSIONS
153
154 updated = False
155 if new_type != self.difference_type:
156 updated = True
157 self.difference_type = new_type
158
159 version = self.source_version
160 parent_version = self.parent_source_version
161 if self.status == DistroSeriesDifferenceStatus.RESOLVED:
162 if version != parent_version:
163 updated = True
164 self.status = DistroSeriesDifferenceStatus.NEEDS_ATTENTION
165 else:
166 if version == parent_version:
167 updated = True
168 self.status = DistroSeriesDifferenceStatus.RESOLVED
169
170 return updated
134171
135 def addComment(self, owner, comment):172 def addComment(self, owner, comment):
136 """See `IDistroSeriesDifference`."""173 """See `IDistroSeriesDifference`."""
137 return getUtility(IDistroSeriesDifferenceCommentSource).new(174 return getUtility(IDistroSeriesDifferenceCommentSource).new(
138 self, owner, comment)175 self, owner, comment)
176
177 def getComments(self):
178 """See `IDistroSeriesDifference`."""
179 DSDComment = DistroSeriesDifferenceComment
180 comments = IStore(DSDComment).find(
181 DistroSeriesDifferenceComment,
182 DSDComment.distro_series_difference == self)
183 return comments.order_by(DSDComment.id)
139184
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-08-31 15:59:47 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-08-31 15:59:49 +0000
@@ -3,17 +3,27 @@
33
4"""Model tests for the DistroSeriesDifference class."""4"""Model tests for the DistroSeriesDifference class."""
55
6from __future__ import with_statement
7
6__metaclass__ = type8__metaclass__ = type
79
8import unittest10import unittest
911
10from storm.store import Store12from storm.store import Store
11from zope.component import getUtility13from zope.component import getUtility
14from zope.security.interfaces import Unauthorized
1215
16from canonical.launchpad.webapp.authorization import check_permission
13from canonical.launchpad.webapp.testing import verifyObject17from canonical.launchpad.webapp.testing import verifyObject
14from canonical.testing import DatabaseFunctionalLayer18from canonical.testing import DatabaseFunctionalLayer
15from lp.testing import TestCaseWithFactory19from lp.testing import (
16from lp.registry.enum import DistroSeriesDifferenceType20 person_logged_in,
21 TestCaseWithFactory,
22 )
23from lp.registry.enum import (
24 DistroSeriesDifferenceStatus,
25 DistroSeriesDifferenceType,
26 )
17from lp.registry.exceptions import NotADerivedSeriesError27from lp.registry.exceptions import NotADerivedSeriesError
18from lp.registry.interfaces.distroseriesdifference import (28from lp.registry.interfaces.distroseriesdifference import (
19 IDistroSeriesDifference,29 IDistroSeriesDifference,
@@ -105,6 +115,116 @@
105115
106 self.assertEqual(pending_pub, ds_diff.parent_source_pub)116 self.assertEqual(pending_pub, ds_diff.parent_source_pub)
107117
118 def test_source_version(self):
119 # The version of the source in the derived series is returned.
120 ds_diff = self.factory.makeDistroSeriesDifference(
121 source_package_name_str="foonew")
122
123 self.assertEqual(
124 ds_diff.source_pub.source_package_version, ds_diff.source_version)
125
126 def test_source_version_none(self):
127 # None is returned for source_version when there is no source pub.
128 ds_diff = self.factory.makeDistroSeriesDifference(
129 source_package_name_str="foonew",
130 difference_type=(
131 DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES))
132
133 self.assertEqual(None, ds_diff.source_version)
134
135 def test_updateStatusAndType_resolves_difference(self):
136 # Status is set to resolved when versions match.
137 ds_diff = self.factory.makeDistroSeriesDifference(
138 source_package_name_str="foonew",
139 versions={
140 'parent': '1.0',
141 'derived': '0.9',
142 })
143 new_derived_pub = self.factory.makeSourcePackagePublishingHistory(
144 sourcepackagename=ds_diff.source_package_name,
145 distroseries=ds_diff.derived_series,
146 status=PackagePublishingStatus.PENDING,
147 version='1.0')
148
149 was_updated = ds_diff.updateStatusAndType()
150
151 self.assertTrue(was_updated)
152 self.assertEqual(
153 DistroSeriesDifferenceStatus.RESOLVED,
154 ds_diff.status)
155
156 def test_updateStatusAndType_re_opens_difference(self):
157 # The status of a resolved difference will updated with new
158 # uploads.
159 ds_diff = self.factory.makeDistroSeriesDifference(
160 source_package_name_str="foonew",
161 versions={
162 'parent': '1.0',
163 'derived': '1.0',
164 },
165 status=DistroSeriesDifferenceStatus.RESOLVED)
166 new_derived_pub = self.factory.makeSourcePackagePublishingHistory(
167 sourcepackagename=ds_diff.source_package_name,
168 distroseries=ds_diff.derived_series,
169 status=PackagePublishingStatus.PENDING,
170 version='1.1')
171
172 was_updated = ds_diff.updateStatusAndType()
173
174 self.assertTrue(was_updated)
175 self.assertEqual(
176 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
177 ds_diff.status)
178
179 def test_updateStatusAndType_new_version_no_change(self):
180 # Uploading a new (different) version does not necessarily
181 # update the record.
182 # In this case, a new version is uploaded, but there is still a
183 # difference needing attention.
184 ds_diff = self.factory.makeDistroSeriesDifference(
185 source_package_name_str="foonew",
186 versions={
187 'parent': '1.0',
188 'derived': '0.9',
189 })
190 new_derived_pub = self.factory.makeSourcePackagePublishingHistory(
191 sourcepackagename=ds_diff.source_package_name,
192 distroseries=ds_diff.derived_series,
193 status=PackagePublishingStatus.PENDING,
194 version='1.1')
195
196 was_updated = ds_diff.updateStatusAndType()
197
198 self.assertFalse(was_updated)
199 self.assertEqual(
200 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
201 ds_diff.status)
202
203 def test_updateStatusAndType_changes_type(self):
204 # The type of difference is updated when appropriate.
205 # In this case, a package that was previously only in the
206 # derived series (UNIQUE_TO_DERIVED_SERIES), is uploaded
207 # to the parent series with a different version.
208 ds_diff = self.factory.makeDistroSeriesDifference(
209 source_package_name_str="foonew",
210 versions={
211 'derived': '0.9',
212 },
213 difference_type=(
214 DistroSeriesDifferenceType.UNIQUE_TO_DERIVED_SERIES))
215 new_parent_pub = self.factory.makeSourcePackagePublishingHistory(
216 sourcepackagename=ds_diff.source_package_name,
217 distroseries=ds_diff.derived_series.parent_series,
218 status=PackagePublishingStatus.PENDING,
219 version='1.1')
220
221 was_updated = ds_diff.updateStatusAndType()
222
223 self.assertTrue(was_updated)
224 self.assertEqual(
225 DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
226 ds_diff.difference_type)
227
108 def test_title(self):228 def test_title(self):
109 # The title is a friendly description of the difference.229 # The title is a friendly description of the difference.
110 parent_series = self.factory.makeDistroSeries(name="lucid")230 parent_series = self.factory.makeDistroSeries(name="lucid")
@@ -126,11 +246,45 @@
126 # Adding a comment creates a new DistroSeriesDifferenceComment246 # Adding a comment creates a new DistroSeriesDifferenceComment
127 ds_diff = self.factory.makeDistroSeriesDifference(247 ds_diff = self.factory.makeDistroSeriesDifference(
128 source_package_name_str="foonew")248 source_package_name_str="foonew")
249
250 with person_logged_in(ds_diff.owner):
251 dsd_comment = ds_diff.addComment(
252 ds_diff.owner, "Wait until version 2.1")
253
254 self.assertEqual(ds_diff, dsd_comment.distro_series_difference)
255
256 def test_getComments(self):
257 # All comments for this difference are returned.
258 ds_diff = self.factory.makeDistroSeriesDifference()
259
260 with person_logged_in(ds_diff.owner):
261 dsd_comment = ds_diff.addComment(
262 ds_diff.owner, "Wait until version 2.1")
263 dsd_comment_2 = ds_diff.addComment(
264 ds_diff.owner, "Wait until version 2.1")
265
266 self.assertEqual(
267 [dsd_comment, dsd_comment_2], list(ds_diff.getComments()))
268
269 def test_addComment_not_public(self):
270 # Comments cannot be added with launchpad.View.
271 ds_diff = self.factory.makeDistroSeriesDifference()
129 person = self.factory.makePerson()272 person = self.factory.makePerson()
130273
131 dsd_comment = ds_diff.addComment(person, "Wait until version 2.1")274 with person_logged_in(person):
132275 self.assertTrue(check_permission('launchpad.View', ds_diff))
133 self.assertEqual(ds_diff, dsd_comment.distro_series_difference)276 self.assertFalse(check_permission('launchpad.Edit', ds_diff))
277 self.assertRaises(Unauthorized, getattr, ds_diff, 'addComment')
278
279 def test_addComment_for_owners(self):
280 # Comments can be added by any of the owners of the derived
281 # series.
282 ds_diff = self.factory.makeDistroSeriesDifference()
283
284 with person_logged_in(ds_diff.owner):
285 self.assertTrue(check_permission('launchpad.Edit', ds_diff))
286 diff_comment = ds_diff.addComment(
287 ds_diff.derived_series.owner, "Boo")
134288
135289
136def test_suite():290def test_suite():
137291
=== modified file 'lib/lp/registry/tests/test_distroseriesdifferencecomment.py'
--- lib/lp/registry/tests/test_distroseriesdifferencecomment.py 2010-08-31 15:59:47 +0000
+++ lib/lp/registry/tests/test_distroseriesdifferencecomment.py 2010-08-31 15:59:49 +0000
@@ -6,11 +6,13 @@
6__metaclass__ = type6__metaclass__ = type
77
8from storm.store import Store8from storm.store import Store
9from zope.component import getUtility
910
10from canonical.launchpad.webapp.testing import verifyObject11from canonical.launchpad.webapp.testing import verifyObject
11from canonical.testing import DatabaseFunctionalLayer12from canonical.testing import DatabaseFunctionalLayer
12from lp.registry.interfaces.distroseriesdifferencecomment import (13from lp.registry.interfaces.distroseriesdifferencecomment import (
13 IDistroSeriesDifferenceComment,14 IDistroSeriesDifferenceComment,
15 IDistroSeriesDifferenceCommentSource,
14 )16 )
15from lp.testing import TestCaseWithFactory17from lp.testing import TestCaseWithFactory
1618
1719
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-08-31 15:59:47 +0000
+++ lib/lp/testing/factory.py 2010-08-31 15:59:49 +0000
@@ -154,7 +154,10 @@
154 IHWSubmissionDeviceSet,154 IHWSubmissionDeviceSet,
155 IHWSubmissionSet,155 IHWSubmissionSet,
156 )156 )
157from lp.registry.enum import DistroSeriesDifferenceType157from lp.registry.enum import (
158 DistroSeriesDifferenceStatus,
159 DistroSeriesDifferenceType,
160 )
158from lp.registry.interfaces.distribution import IDistributionSet161from lp.registry.interfaces.distribution import IDistributionSet
159from lp.registry.interfaces.distributionmirror import (162from lp.registry.interfaces.distributionmirror import (
160 MirrorContent,163 MirrorContent,
@@ -1827,7 +1830,8 @@
1827 def makeDistroSeriesDifference(1830 def makeDistroSeriesDifference(
1828 self, derived_series=None, source_package_name_str=None,1831 self, derived_series=None, source_package_name_str=None,
1829 versions=None,1832 versions=None,
1830 difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS):1833 difference_type=DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
1834 status=DistroSeriesDifferenceStatus.NEEDS_ATTENTION):
1831 """Create a new distro series source package difference."""1835 """Create a new distro series source package difference."""
1832 if derived_series is None:1836 if derived_series is None:
1833 parent_series = self.makeDistroSeries()1837 parent_series = self.makeDistroSeries()
@@ -1860,7 +1864,8 @@
1860 sourcepackagename=source_package_name)1864 sourcepackagename=source_package_name)
18611865
1862 return getUtility(IDistroSeriesDifferenceSource).new(1866 return getUtility(IDistroSeriesDifferenceSource).new(
1863 derived_series, source_package_name, difference_type)1867 derived_series, source_package_name, difference_type,
1868 status=status)
18641869
1865 def makeDistroSeriesDifferenceComment(1870 def makeDistroSeriesDifferenceComment(
1866 self, distro_series_difference=None, owner=None, comment=None):1871 self, distro_series_difference=None, owner=None, comment=None):

Subscribers

People subscribed via source and target branches

to status/vote changes: