Merge ~pappacena/launchpad:change-override-log into launchpad:master
- Git
- lp:~pappacena/launchpad
- change-override-log
- Merge into master
Status: | Merged |
---|---|
Approved by: | Thiago F. Pappacena |
Approved revision: | 66fa2e6d4ae5720963eeb101e292584d33038254 |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~pappacena/launchpad:change-override-log |
Merge into: | launchpad:master |
Diff against target: |
331 lines (+131/-10) 7 files modified
lib/lp/soyuz/interfaces/publishing.py (+13/-3) lib/lp/soyuz/model/publishing.py (+11/-3) lib/lp/soyuz/stories/soyuz/xx-packagepublishinghistory.txt (+27/-0) lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt (+1/-0) lib/lp/soyuz/templates/packagepublishing-details.pt (+8/-0) lib/lp/soyuz/tests/test_publishing.py (+67/-2) lib/lp/testing/factory.py (+4/-2) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+380252@code.launchpad.net |
Commit message
Saving and showing the user that requested to override a package publishing.
Description of the change
Thiago F. Pappacena (pappacena) wrote : | # |
Colin Watson (cjwatson) wrote : | # |
This looks like broadly the right sort of direction. The main thing that I think needs to change is to explicitly pass the principal down from the caller (using @call_with(
Colin Watson (cjwatson) : | # |
Thiago F. Pappacena (pappacena) wrote : | # |
Added more info about why it's showing only for PENDING, cjwatson. Let me know what do you think about showing in every other situation.
I'm doing the other requests right now.
Colin Watson (cjwatson) : | # |
Thiago F. Pappacena (pappacena) wrote : | # |
cjwatson, pushed the requested change. It should now show the "Created by ..." always, apart from deleted and copied packages, which are special cases.
Colin Watson (cjwatson) : | # |
Thiago F. Pappacena (pappacena) wrote : | # |
Changes pushed. It should good to go as soon as we deploy the database change to production.
Colin Watson (cjwatson) wrote : | # |
This can land now - the database patch is in place.
Thiago F. Pappacena (pappacena) wrote : | # |
Thanks! I'll top-approve it now.
Preview Diff
1 | diff --git a/lib/lp/soyuz/interfaces/publishing.py b/lib/lp/soyuz/interfaces/publishing.py |
2 | index 02c3615..4697209 100644 |
3 | --- a/lib/lp/soyuz/interfaces/publishing.py |
4 | +++ b/lib/lp/soyuz/interfaces/publishing.py |
5 | @@ -1,4 +1,4 @@ |
6 | -# Copyright 2009-2019 Canonical Ltd. This software is licensed under the |
7 | +# Copyright 2009-2020 Canonical Ltd. This software is licensed under the |
8 | # GNU Affero General Public License version 3 (see the file LICENSE). |
9 | |
10 | """Publishing interfaces.""" |
11 | @@ -561,8 +561,9 @@ class ISourcePackagePublishingHistoryEdit(IPublishingEdit): |
12 | new_component=TextLine(title=u"The new component name."), |
13 | new_section=TextLine(title=u"The new section name.")) |
14 | @export_write_operation() |
15 | + @call_with(creator=REQUEST_USER) |
16 | @operation_for_version("devel") |
17 | - def changeOverride(new_component=None, new_section=None): |
18 | + def changeOverride(new_component=None, new_section=None, creator=None): |
19 | """Change the component and/or section of this publication. |
20 | |
21 | It is changed only if the argument is not None. |
22 | @@ -663,6 +664,13 @@ class IBinaryPackagePublishingHistoryPublic(IPublishingView): |
23 | title=_('The build which superseded this one'), |
24 | required=False, readonly=False, |
25 | ) |
26 | + creator = exported( |
27 | + Reference( |
28 | + IPerson, |
29 | + title=_('Publication Creator'), |
30 | + description=_('The IPerson who created this publication.'), |
31 | + required=False, readonly=True |
32 | + )) |
33 | datecreated = exported( |
34 | Datetime( |
35 | title=_('Date Created'), |
36 | @@ -840,9 +848,11 @@ class IBinaryPackagePublishingHistoryEdit(IPublishingEdit): |
37 | new_phased_update_percentage=Int( |
38 | title=u"The new phased update percentage.")) |
39 | @export_write_operation() |
40 | + @call_with(creator=REQUEST_USER) |
41 | @operation_for_version("devel") |
42 | def changeOverride(new_component=None, new_section=None, |
43 | - new_priority=None, new_phased_update_percentage=None): |
44 | + new_priority=None, new_phased_update_percentage=None, |
45 | + creator=None): |
46 | """Change the component/section/priority/phase of this publication. |
47 | |
48 | It is changed only if the argument is not None. |
49 | diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py |
50 | index 3e73e98..ae7b414 100644 |
51 | --- a/lib/lp/soyuz/model/publishing.py |
52 | +++ b/lib/lp/soyuz/model/publishing.py |
53 | @@ -1,4 +1,4 @@ |
54 | -# Copyright 2009-2019 Canonical Ltd. This software is licensed under the |
55 | +# Copyright 2009-2020 Canonical Ltd. This software is licensed under the |
56 | # GNU Affero General Public License version 3 (see the file LICENSE). |
57 | |
58 | __metaclass__ = type |
59 | @@ -471,7 +471,8 @@ class SourcePackagePublishingHistory(SQLBase, ArchivePublisherBase): |
60 | |
61 | self.supersededby = dominant.sourcepackagerelease |
62 | |
63 | - def changeOverride(self, new_component=None, new_section=None): |
64 | + def changeOverride(self, new_component=None, new_section=None, |
65 | + creator=None): |
66 | """See `ISourcePackagePublishingHistory`.""" |
67 | # Check we have been asked to do something |
68 | if (new_component is None and |
69 | @@ -516,6 +517,7 @@ class SourcePackagePublishingHistory(SQLBase, ArchivePublisherBase): |
70 | pocket=self.pocket, |
71 | component=new_component, |
72 | section=new_section, |
73 | + creator=creator, |
74 | archive=self.archive) |
75 | |
76 | def copyTo(self, distroseries, pocket, archive, override=None, |
77 | @@ -629,6 +631,9 @@ class BinaryPackagePublishingHistory(SQLBase, ArchivePublisherBase): |
78 | phased_update_percentage = IntCol( |
79 | dbName='phased_update_percentage', notNull=False, default=None) |
80 | scheduleddeletiondate = UtcDateTimeCol(default=None) |
81 | + creator = ForeignKey( |
82 | + dbName='creator', foreignKey='Person', |
83 | + storm_validator=validate_public_person, notNull=False, default=None) |
84 | datepublished = UtcDateTimeCol(default=None) |
85 | datecreated = UtcDateTimeCol(default=UTC_NOW) |
86 | datesuperseded = UtcDateTimeCol(default=None) |
87 | @@ -794,7 +799,8 @@ class BinaryPackagePublishingHistory(SQLBase, ArchivePublisherBase): |
88 | dominated.supersede(dominant, logger) |
89 | |
90 | def changeOverride(self, new_component=None, new_section=None, |
91 | - new_priority=None, new_phased_update_percentage=None): |
92 | + new_priority=None, new_phased_update_percentage=None, |
93 | + creator=None): |
94 | """See `IBinaryPackagePublishingHistory`.""" |
95 | |
96 | # Check we have been asked to do something |
97 | @@ -873,6 +879,7 @@ class BinaryPackagePublishingHistory(SQLBase, ArchivePublisherBase): |
98 | component=new_component, |
99 | section=new_section, |
100 | priority=new_priority, |
101 | + creator=creator, |
102 | archive=debug.archive, |
103 | phased_update_percentage=new_phased_update_percentage) |
104 | |
105 | @@ -888,6 +895,7 @@ class BinaryPackagePublishingHistory(SQLBase, ArchivePublisherBase): |
106 | section=new_section, |
107 | priority=new_priority, |
108 | archive=self.archive, |
109 | + creator=creator, |
110 | phased_update_percentage=new_phased_update_percentage) |
111 | |
112 | def copyTo(self, distroseries, pocket, archive): |
113 | diff --git a/lib/lp/soyuz/stories/soyuz/xx-packagepublishinghistory.txt b/lib/lp/soyuz/stories/soyuz/xx-packagepublishinghistory.txt |
114 | index 31e98b7..51a629b 100644 |
115 | --- a/lib/lp/soyuz/stories/soyuz/xx-packagepublishinghistory.txt |
116 | +++ b/lib/lp/soyuz/stories/soyuz/xx-packagepublishinghistory.txt |
117 | @@ -23,10 +23,35 @@ shows the complete history of a package in all series. |
118 | >>> print(extract_text(table)) |
119 | Date Status Target Pocket Component Section Version |
120 | ... UTC Published Breezy ... release main base 666 |
121 | + Created ... ago by Foo Bar |
122 | Published ... ago |
123 | >>> print(table.findAll("tr")[2].td["colspan"]) |
124 | 8 |
125 | |
126 | +A change-override request should show who made the request |
127 | + |
128 | + >>> from lp.registry.interfaces.person import IPersonSet |
129 | + >>> from zope.component import getUtility |
130 | + |
131 | + >>> login('foo.bar@canonical.com') |
132 | + >>> person = getUtility(IPersonSet).getByEmail('foo.bar@canonical.com') |
133 | + >>> new_pub = source_pub.changeOverride( |
134 | + ... new_component='universe', creator=person) |
135 | + >>> logout() |
136 | + |
137 | + >>> anon_browser.open( |
138 | + ... 'http://launchpad.test/ubuntutest/+source/test-history/' |
139 | + ... '+publishinghistory') |
140 | + >>> table = find_tag_by_id(anon_browser.contents, 'publishing-summary') |
141 | + >>> print(extract_text(table)) |
142 | + Date Status Target Pocket Component Section Version |
143 | + ... UTC Pending Breezy ... release universe base 666 |
144 | + Created ... ago by Foo Bar |
145 | + ... UTC Published Breezy ... release main base 666 |
146 | + Created ... ago by Foo Bar |
147 | + Published ... ago |
148 | + |
149 | + |
150 | A publishing record will be shown as deleted in the publishing history after a |
151 | request for deletion by a user. |
152 | |
153 | @@ -42,6 +67,8 @@ request for deletion by a user. |
154 | >>> table = find_tag_by_id(anon_browser.contents, 'publishing-summary') |
155 | >>> print(extract_text(table)) |
156 | Date Status Target Pocket Component Section Version |
157 | + ... UTC Pending Breezy ... release universe base 666 |
158 | + Created ... ago by Foo Bar |
159 | Deleted Breezy ... release main base 666 |
160 | Deleted ... ago by ... fix bug 1 |
161 | Published ... ago |
162 | diff --git a/lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt b/lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt |
163 | index 81482f2..722c4c0 100644 |
164 | --- a/lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt |
165 | +++ b/lib/lp/soyuz/stories/webservice/xx-binary-package-publishing.txt |
166 | @@ -71,6 +71,7 @@ Each binary publication exposes a number of properties: |
167 | binary_package_version: u'1.0' |
168 | build_link: u'http://.../~cprov/+archive/ubuntu/ppa/+build/28' |
169 | component_name: u'main' |
170 | + creator_link: None |
171 | date_created: u'2007-08-10T13:00:00+00:00' |
172 | date_made_pending: None |
173 | date_published: u'2007-08-10T13:00:01+00:00' |
174 | diff --git a/lib/lp/soyuz/templates/packagepublishing-details.pt b/lib/lp/soyuz/templates/packagepublishing-details.pt |
175 | index 0b0102c..4c65323 100644 |
176 | --- a/lib/lp/soyuz/templates/packagepublishing-details.pt |
177 | +++ b/lib/lp/soyuz/templates/packagepublishing-details.pt |
178 | @@ -4,6 +4,14 @@ |
179 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
180 | omit-tag=""> |
181 | <ul> |
182 | + <li tal:condition="python: context.creator |
183 | + and not view.wasCopied() |
184 | + and not view.wasDeleted()"> |
185 | + Created |
186 | + <span tal:attributes="title context/datecreated/fmt:datetime" |
187 | + tal:content="context/datecreated/fmt:displaydate" /> |
188 | + by <a tal:replace="structure context/creator/fmt:link"/> |
189 | + </li> |
190 | <li tal:condition="view/isRemoved"> |
191 | Removed from disk |
192 | <span tal:attributes="title context/dateremoved/fmt:datetime" |
193 | diff --git a/lib/lp/soyuz/tests/test_publishing.py b/lib/lp/soyuz/tests/test_publishing.py |
194 | index c29028e..e65003d 100644 |
195 | --- a/lib/lp/soyuz/tests/test_publishing.py |
196 | +++ b/lib/lp/soyuz/tests/test_publishing.py |
197 | @@ -1,4 +1,4 @@ |
198 | -# Copyright 2009-2019 Canonical Ltd. This software is licensed under the |
199 | +# Copyright 2009-2020 Canonical Ltd. This software is licensed under the |
200 | # GNU Affero General Public License version 3 (see the file LICENSE). |
201 | |
202 | """Test native publication workflow for Soyuz. """ |
203 | @@ -62,6 +62,9 @@ from lp.soyuz.model.publishing import ( |
204 | SourcePackagePublishingHistory, |
205 | ) |
206 | from lp.testing import ( |
207 | + login_admin, |
208 | + person_logged_in, |
209 | + record_two_runs, |
210 | StormStatementRecorder, |
211 | TestCaseWithFactory, |
212 | ) |
213 | @@ -72,6 +75,7 @@ from lp.testing.dbuser import ( |
214 | from lp.testing.factory import LaunchpadObjectFactory |
215 | from lp.testing.layers import ( |
216 | DatabaseFunctionalLayer, |
217 | + LaunchpadFunctionalLayer, |
218 | LaunchpadZopelessLayer, |
219 | ZopelessDatabaseLayer, |
220 | ) |
221 | @@ -997,13 +1001,15 @@ class TestPublishingSetLite(TestCaseWithFactory): |
222 | debug_non_match_bpph.status, PackagePublishingStatus.PENDING) |
223 | |
224 | def test_changeOverride_also_overrides_debug_package(self): |
225 | + user = self.factory.makePerson() |
226 | bpph, debug_bpph = self.factory.makeBinaryPackagePublishingHistory( |
227 | pocket=PackagePublishingPocket.RELEASE, with_debug=True) |
228 | new_section = self.factory.makeSection() |
229 | - new_bpph = bpph.changeOverride(new_section=new_section) |
230 | + new_bpph = bpph.changeOverride(new_section=new_section, creator=user) |
231 | publishing_set = getUtility(IPublishingSet) |
232 | [new_debug_bpph] = publishing_set.findCorrespondingDDEBPublications( |
233 | [new_bpph]) |
234 | + self.assertEqual(new_debug_bpph.creator, user) |
235 | self.assertEqual(new_debug_bpph.section, new_section) |
236 | |
237 | def test_requestDeletion_forbids_debug_package(self): |
238 | @@ -1524,6 +1530,21 @@ class TestChangeOverride(TestNativePublishingBase): |
239 | binary=True, new_component="universe", new_section="misc", |
240 | new_priority="extra", new_phased_update_percentage=90) |
241 | |
242 | + def test_change_binary_logged_in_user(self): |
243 | + person = self.factory.makePerson() |
244 | + new_pub = self.assertCanOverride( |
245 | + binary=True, new_component="universe", new_section="misc", |
246 | + new_priority="extra", new_phased_update_percentage=90, |
247 | + creator=person) |
248 | + self.assertEqual(person, new_pub.creator) |
249 | + |
250 | + def test_change_source_logged_in_user(self): |
251 | + person = self.factory.makePerson() |
252 | + new_pub = self.assertCanOverride( |
253 | + binary=False, new_component="universe", new_section="misc", |
254 | + creator=person) |
255 | + self.assertEqual(person, new_pub.creator) |
256 | + |
257 | def test_set_and_clear_phased_update_percentage(self): |
258 | # new_phased_update_percentage=<integer> sets a phased update |
259 | # percentage; new_phased_update_percentage=100 clears it. |
260 | @@ -1572,3 +1593,47 @@ class TestChangeOverride(TestNativePublishingBase): |
261 | # archive. |
262 | self.assertCannotOverride(new_component="partner") |
263 | self.assertCannotOverride(binary=True, new_component="partner") |
264 | + |
265 | + |
266 | +class TestPublishingHistoryView(TestCaseWithFactory): |
267 | + layer = LaunchpadFunctionalLayer |
268 | + |
269 | + def test_constant_query_counts_on_publishing_history_change_override(self): |
270 | + admin = self.factory.makeAdministrator() |
271 | + normal_user = self.factory.makePerson() |
272 | + |
273 | + with person_logged_in(admin): |
274 | + test_publisher = SoyuzTestPublisher() |
275 | + test_publisher.prepareBreezyAutotest() |
276 | + |
277 | + source_pub = test_publisher.getPubSource( |
278 | + "test-history", status=PackagePublishingStatus.PUBLISHED) |
279 | + url = ("http://launchpad.test/ubuntutest/+source/test-history" |
280 | + "/+publishinghistory") |
281 | + |
282 | + def insert_more_publish_history(): |
283 | + person1 = self.factory.makePerson() |
284 | + new_component = ( |
285 | + 'universe' if source_pub.component.name == 'main' |
286 | + else 'main') |
287 | + source_pub.changeOverride( |
288 | + new_component=new_component, creator=person1) |
289 | + |
290 | + person2 = self.factory.makePerson() |
291 | + new_section = ('web' if source_pub.section.name == 'base' |
292 | + else 'base') |
293 | + source_pub.changeOverride( |
294 | + new_section=new_section, creator=person2) |
295 | + |
296 | + def show_page(): |
297 | + self.getUserBrowser(url, normal_user) |
298 | + |
299 | + # Make sure to have all the history fitting in one page. |
300 | + self.pushConfig("launchpad", default_batch_size=50) |
301 | + |
302 | + recorder1, recorder2 = record_two_runs( |
303 | + show_page, insert_more_publish_history, |
304 | + 1, 10, login_method=login_admin) |
305 | + |
306 | + self.assertThat(recorder1, HasQueryCount(Equals(26))) |
307 | + self.assertThat(recorder2, HasQueryCount.byEquality(recorder1)) |
308 | diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py |
309 | index 8248fa8..2bdac13 100644 |
310 | --- a/lib/lp/testing/factory.py |
311 | +++ b/lib/lp/testing/factory.py |
312 | @@ -3989,7 +3989,8 @@ class BareLaunchpadObjectFactory(ObjectFactory): |
313 | sourcepackagename=None, |
314 | version=None, |
315 | architecturespecific=False, |
316 | - with_debug=False, with_file=False): |
317 | + with_debug=False, with_file=False, |
318 | + creator=None): |
319 | """Make a `BinaryPackagePublishingHistory`.""" |
320 | if distroarchseries is None: |
321 | if archive is None: |
322 | @@ -4082,7 +4083,8 @@ class BareLaunchpadObjectFactory(ObjectFactory): |
323 | binpackageformat=BinaryPackageFormat.DDEB, |
324 | sourcepackagename=sourcepackagename, |
325 | architecturespecific=architecturespecific, |
326 | - with_file=with_file) |
327 | + with_file=with_file, |
328 | + creator=creator) |
329 | removeSecurityProxy(bpph.binarypackagerelease).debug_package = ( |
330 | debug_bpph.binarypackagerelease) |
331 | return bpphs[0], debug_bpph |
cjwatson, this is still a WIP (I'm still missing tests, for example), but I would like to get a "pre-review", just to make sure we are aligned on what should be done here.
Basically, I checked lp:ubuntu-archive-tools repo and it seems that the `change-override` scripts call the changeOverride on the API, with an authenticated user.
So, on LP side, I'm getting the logged in user and setting it on the "creator" field both for Source and Binary PackagePublishi ngHistory. Then, showing it on the details of the +publishinghistory page.