Merge lp:~kfogel/launchpad/506018-patch-report into lp:launchpad
- 506018-patch-report
- Merge into devel
Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp:~kfogel/launchpad/506018-patch-report |
Merge into: | lp:launchpad |
Diff against target: |
743 lines (+567/-9) 9 files modified
lib/canonical/launchpad/icing/style.css (+5/-1) lib/lp/bugs/browser/bugtarget.py (+74/-2) lib/lp/bugs/browser/configure.zcml (+8/-1) lib/lp/bugs/interfaces/bugtarget.py (+1/-1) lib/lp/bugs/interfaces/bugtask.py (+4/-0) lib/lp/bugs/model/bugtarget.py (+1/-1) lib/lp/bugs/stories/patches-view/patches-view.txt (+323/-0) lib/lp/bugs/templates/bugtarget-patches.pt (+125/-0) lib/lp/testing/factory.py (+26/-3) |
To merge this branch: | bzr merge lp:~kfogel/launchpad/506018-patch-report |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Karl Fogel (community) | Approve | ||
Martin Albisetti (community) | ui | Approve | |
Abel Deuring (community) | code | Approve | |
Eleanor Berger (community) | code | Approve | |
Review via email: mp+18181@code.launchpad.net |
Commit message
Description of the change
Karl Fogel (kfogel) wrote : | # |
Martin Albisetti (beuno) wrote : | # |
Overall, this branch is fantastic. I know a lot of people will appreciate a lot your work on this.
A few small tweaks discussed on IRC:
- Padding in the tooltips would be super nice
- I'd add an "Order by: " label to the ordering drop down, and drop the "by" from each option to improve readability
- Bug icons don't respect importance
Eleanor Berger (intellectronica) wrote : | # |
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -1458,7 +1458,7 @@
>
> div.popupTitle {
> background: #ffffdc;
> - padding: 0 1em;
> + padding: 0.5em 1em;
> border: 1px black solid;
> position: absolute;
> display: none;
>
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -1,4 +1,4 @@
> -# Copyright 2009 Canonical Ltd. This software is licensed under the
> +# Copyright 2010 Canonical Ltd. This software is licensed under the
> # GNU Affero General Public License version 3 (see the file LICENSE).
>
> """IBugTarget-
> @@ -15,12 +15,15 @@
> "FileBugViewBase",
> "OfficialBugTag
> "ProjectFileBug
> + "BugsPatchesView",
> ]
Let's sort this alphabetically, so that it's easier to locate an export later.
>
> import cgi
> from cStringIO import StringIO
> +from datetime import datetime
> from email import message_from_string
> from operator import itemgetter
> +from pytz import timezone
> from simplejson import dumps
> import tempfile
> import urllib
> @@ -40,6 +43,7 @@
> from canonical.config import config
> from lp.bugs.
> from lp.bugs.
> +from lp.bugs.
> from canonical.
> BugFeedLink, BugTargetLatest
> PersonLatestBug
> @@ -73,6 +77,7 @@
> LaunchpadEditFo
> canonical_url, custom_widget, safe_action)
> from canonical.
> +from canonical.
> from canonical.
> from canonical.
> from canonical.
> @@ -1372,3 +1377,51 @@
> class BugsVHostBreadc
> rootsite = 'bugs'
> text = 'Bugs'
> +
> +
> +class BugsPatchesView
> + """View list of patch attachments associated with bugs."""
> +
> + @property
> + def label(self):
> + """The display label for the view."""
> + return 'Patch attachments in %s' % self.context.
> +
> + def batchedPatchTas
> + """Return a BatchNavigator for bug tasks with patch attachments."""
> + any_unresolved_
> + UNRESOLVED_
We avoid using line continuation in Launchpad code, because we don't like how
it messes with the interpretation of the text file (the parser consideres
everything after the \ to actually be on the same line). Instead we use
parentheses, like this:
Abel Deuring (adeuring) wrote : | # |
On 28.01.2010 19:23, Tom Berger wrote:
>> === added directory 'lib/lp/
>> === added file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -0,0 +1,301 @@
>> +Patches View
>> +============
>> +
>> +Patches View by Product
>> +------
>> +
>> +We have a view listing patches attached to bugs that target a given
>> +product. At first, the product is new and has no bugs.
>> +
>> + >>> def make_thing(
>> + ... login('<email address hidden>')
>> + ... result = factory_
>> + ... transaction.
>> + ... logout()
>> + ... return result
>
> That's a nice helper function. Let's extract it out of this test and make it
> generally available to all tests. It probably deserves a more descriptive name,
> though.
A generally available function should have a way to specify the user
that should login: Some objects can only be created by script users.
And this is actually a good candidate for using the "with" statement --
provided that Jeroen does not object to using it for "transaction
wrapping". (see the notes for last reviewers meeting. Has Barry posted
them already?)
Something like
with factory.
factory.
would look much nicer than
make_
Abel
Eleanor Berger (intellectronica) wrote : | # |
On 28 January 2010 19:51, Abel Deuring <email address hidden> wrote:
> On 28.01.2010 19:23, Tom Berger wrote:
>>> === added directory 'lib/lp/
>>> === added file 'lib/lp/
>>> --- lib/lp/
>>> +++ lib/lp/
>>> @@ -0,0 +1,301 @@
>>> +Patches View
>>> +============
>>> +
>>> +Patches View by Product
>>> +------
>>> +
>>> +We have a view listing patches attached to bugs that target a given
>>> +product. At first, the product is new and has no bugs.
>>> +
>>> + >>> def make_thing(
>>> + ... login('<email address hidden>')
>>> + ... result = factory_
>>> + ... transaction.
>>> + ... logout()
>>> + ... return result
>>
>> That's a nice helper function. Let's extract it out of this test and make it
>> generally available to all tests. It probably deserves a more descriptive name,
>> though.
>
> A generally available function should have a way to specify the user
> that should login: Some objects can only be created by script users.
>
> And this is actually a good candidate for using the "with" statement --
> provided that Jeroen does not object to using it for "transaction
> wrapping". (see the notes for last reviewers meeting. Has Barry posted
> them already?)
>
> Something like
>
> with factory.
> factory.
>
> would look much nicer than
>
> make_thing(
+1000000
Karl Fogel (kfogel) wrote : | # |
Well, doing the Python 'with' thing in story test code proved problematic. But otherwise, all comments have been incorporated now, and pushed up. Thanks, guys.
Karl Fogel (kfogel) wrote : | # |
Note that Michael Hudson effectively reviewed my re-submission when he reviewed
https:/
because he commented on my changes from here (which had been merged into that branch) as well as Tom's. I'm not sure he found any problems with Tom's, actually :-).
So, before reviewing this, please see Michael's comments; he may have already said what you were going to say.
Eleanor Berger (intellectronica) wrote : | # |
You've addressed all of my concerns and the code looks great now.
Abel Deuring (adeuring) wrote : | # |
Hi Karl,
I looked at the changes between revision 10189 and 10194 -- everything is fine.
Abel Deuring (adeuring) wrote : | # |
recent screenshots for the UI review:
http://
http://
http://
http://
http://
Martin Albisetti (beuno) wrote : | # |
A wonderful piece of work. The sooner this lands, the better.
Karl Fogel (kfogel) wrote : | # |
LGTM too now, not that I'm a reviewer, and it's mostly my own change.
Preview Diff
1 | === modified file 'lib/canonical/launchpad/icing/style.css' |
2 | --- lib/canonical/launchpad/icing/style.css 2010-01-21 22:07:42 +0000 |
3 | +++ lib/canonical/launchpad/icing/style.css 2010-02-02 18:46:23 +0000 |
4 | @@ -1237,6 +1237,10 @@ |
5 | margin: 0; |
6 | } |
7 | |
8 | +.patches-view-cell { |
9 | + padding-right: 1em; |
10 | +} |
11 | + |
12 | /* --- Blueprints --- */ |
13 | |
14 | body.tab-specifications #actions, body.tab-specifications .results { |
15 | @@ -1463,7 +1467,7 @@ |
16 | |
17 | div.popupTitle { |
18 | background: #ffffdc; |
19 | - padding: 0 1em; |
20 | + padding: 0.5em 1em; |
21 | border: 1px black solid; |
22 | position: absolute; |
23 | display: none; |
24 | |
25 | === modified file 'lib/lp/bugs/browser/bugtarget.py' |
26 | --- lib/lp/bugs/browser/bugtarget.py 2010-01-29 19:00:47 +0000 |
27 | +++ lib/lp/bugs/browser/bugtarget.py 2010-02-02 18:46:23 +0000 |
28 | @@ -1,4 +1,4 @@ |
29 | -# Copyright 2009 Canonical Ltd. This software is licensed under the |
30 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
31 | # GNU Affero General Public License version 3 (see the file LICENSE). |
32 | |
33 | """IBugTarget-related browser views.""" |
34 | @@ -7,6 +7,7 @@ |
35 | |
36 | __all__ = [ |
37 | "BugsVHostBreadcrumb", |
38 | + "BugsPatchesView", |
39 | "BugTargetBugListingView", |
40 | "BugTargetBugTagsView", |
41 | "BugTargetBugsView", |
42 | @@ -19,8 +20,10 @@ |
43 | |
44 | import cgi |
45 | from cStringIO import StringIO |
46 | +from datetime import datetime |
47 | from email import message_from_string |
48 | from operator import itemgetter |
49 | +from pytz import timezone |
50 | from simplejson import dumps |
51 | import tempfile |
52 | import urllib |
53 | @@ -40,6 +43,7 @@ |
54 | from canonical.config import config |
55 | from lp.bugs.browser.bugtask import BugTaskSearchListingView |
56 | from lp.bugs.interfaces.bug import IBug |
57 | +from lp.bugs.interfaces.bugattachment import BugAttachmentType |
58 | from canonical.launchpad.browser.feeds import ( |
59 | BugFeedLink, BugTargetLatestBugsFeedLink, FeedsMixin, |
60 | PersonLatestBugsFeedLink) |
61 | @@ -48,7 +52,8 @@ |
62 | IBugTarget, IOfficialBugTagTargetPublic, IOfficialBugTagTargetRestricted) |
63 | from lp.bugs.interfaces.bug import IBugSet |
64 | from lp.bugs.interfaces.bugtask import ( |
65 | - BugTaskStatus, IBugTaskSet, UNRESOLVED_BUGTASK_STATUSES) |
66 | + BugTaskStatus, IBugTaskSet, UNRESOLVED_BUGTASK_STATUSES, |
67 | + UNRESOLVED_PLUS_FIXRELEASED_BUGTASK_STATUSES) |
68 | from canonical.launchpad.interfaces.launchpad import ( |
69 | IHasExternalBugTracker, ILaunchpadUsage) |
70 | from canonical.launchpad.interfaces.hwdb import IHWSubmissionSet |
71 | @@ -73,6 +78,7 @@ |
72 | LaunchpadEditFormView, LaunchpadFormView, LaunchpadView, action, |
73 | canonical_url, custom_widget, safe_action) |
74 | from canonical.launchpad.webapp.authorization import check_permission |
75 | +from canonical.launchpad.webapp.batching import BatchNavigator |
76 | from canonical.launchpad.webapp.tales import BugTrackerFormatterAPI |
77 | from canonical.launchpad.validators.name import valid_name_pattern |
78 | from canonical.launchpad.webapp.menu import structured |
79 | @@ -1384,3 +1390,69 @@ |
80 | class BugsVHostBreadcrumb(Breadcrumb): |
81 | rootsite = 'bugs' |
82 | text = 'Bugs' |
83 | + |
84 | + |
85 | +class BugsPatchesView(LaunchpadView): |
86 | + """View list of patch attachments associated with bugs.""" |
87 | + |
88 | + @property |
89 | + def label(self): |
90 | + """The display label for the view.""" |
91 | + return 'Patch attachments in %s' % self.context.displayname |
92 | + |
93 | + def batchedPatchTasks(self): |
94 | + """Return a BatchNavigator for bug tasks with patch attachments.""" |
95 | + # XXX: Karl Fogel 2010-02-01 bug=515584: we should be using a |
96 | + # Zope form instead of validating the values by hand in the |
97 | + # code. Doing it the Zope form way would specify rendering |
98 | + # and validation from the same enum, and thus observe DRY. |
99 | + orderby = self.request.get("orderby") |
100 | + if (orderby is not None and |
101 | + orderby not in ["-importance", "status", "targetname", |
102 | + "datecreated", "-datecreated"]): |
103 | + raise UnexpectedFormData( |
104 | + "Unexpected value for field 'orderby': '%s'" % orderby) |
105 | + return BatchNavigator( |
106 | + self.context.searchTasks( |
107 | + None, user=self.user, order_by=orderby, |
108 | + status=UNRESOLVED_PLUS_FIXRELEASED_BUGTASK_STATUSES, |
109 | + omit_duplicates=True, has_patch=True), |
110 | + self.request) |
111 | + |
112 | + def targetName(self): |
113 | + """Return the name of the current context's target type, or None. |
114 | + |
115 | + The name is something like "Package" or "Project" (meaning |
116 | + Product); it is intended to be appropriate to use as a column |
117 | + name in a web page, for example. If no target type is |
118 | + appropriate for the current context, then return None. |
119 | + """ |
120 | + if (IDistribution.providedBy(self.context) or |
121 | + IDistroSeries.providedBy(self.context)): |
122 | + return "Package" |
123 | + elif IProject.providedBy(self.context): |
124 | + return "Project" # meaning Product |
125 | + else: |
126 | + return None |
127 | + |
128 | + def youngestPatch(self, bug): |
129 | + """Return the youngest patch attached to a bug, else error.""" |
130 | + youngest = None |
131 | + # Loop over bugtasks, gathering youngest patch for each's bug. |
132 | + for attachment in bug.attachments: |
133 | + if attachment.is_patch: |
134 | + if youngest is None: |
135 | + youngest = attachment |
136 | + elif (attachment.message.datecreated > |
137 | + youngest.message.datecreated): |
138 | + youngest = attachment |
139 | + if youngest is None: |
140 | + # This is the patches view, so every bug under |
141 | + # consideration should have at least one patch attachment. |
142 | + raise AssertionError("bug %i has no patch attachments" % bug.id) |
143 | + return youngest |
144 | + |
145 | + def patchAge(self, patch): |
146 | + """Return a timedelta object for the age of a patch attachment.""" |
147 | + now = datetime.now(timezone('UTC')) |
148 | + return now - patch.message.datecreated |
149 | |
150 | === modified file 'lib/lp/bugs/browser/configure.zcml' |
151 | --- lib/lp/bugs/browser/configure.zcml 2010-01-18 21:44:59 +0000 |
152 | +++ lib/lp/bugs/browser/configure.zcml 2010-02-02 18:46:23 +0000 |
153 | @@ -1,4 +1,4 @@ |
154 | -<!-- Copyright 2009 Canonical Ltd. This software is licensed under the |
155 | +<!-- Copyright 2010 Canonical Ltd. This software is licensed under the |
156 | GNU Affero General Public License version 3 (see the file LICENSE). |
157 | --> |
158 | |
159 | @@ -108,6 +108,13 @@ |
160 | facet="bugs" |
161 | permission="launchpad.Edit" |
162 | template="../templates/official-bug-target-manage-tags.pt"/> |
163 | + <browser:page |
164 | + name="+patches" |
165 | + for="lp.bugs.interfaces.bugtarget.IHasBugs" |
166 | + class="lp.bugs.browser.bugtarget.BugsPatchesView" |
167 | + facet="bugs" |
168 | + permission="zope.Public" |
169 | + template="../templates/bugtarget-patches.pt"/> |
170 | </facet> |
171 | <browser:page |
172 | name="+bugtarget-macros-search" |
173 | |
174 | === modified file 'lib/lp/bugs/interfaces/bugtarget.py' |
175 | --- lib/lp/bugs/interfaces/bugtarget.py 2009-08-18 11:12:06 +0000 |
176 | +++ lib/lp/bugs/interfaces/bugtarget.py 2010-02-02 18:46:23 +0000 |
177 | @@ -1,4 +1,4 @@ |
178 | -# Copyright 2009 Canonical Ltd. This software is licensed under the |
179 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
180 | # GNU Affero General Public License version 3 (see the file LICENSE). |
181 | |
182 | # pylint: disable-msg=E0211,E0213 |
183 | |
184 | === modified file 'lib/lp/bugs/interfaces/bugtask.py' |
185 | --- lib/lp/bugs/interfaces/bugtask.py 2010-02-01 22:47:12 +0000 |
186 | +++ lib/lp/bugs/interfaces/bugtask.py 2010-02-02 18:46:23 +0000 |
187 | @@ -36,6 +36,7 @@ |
188 | 'IUpstreamProductBugTaskSearch', |
189 | 'RESOLVED_BUGTASK_STATUSES', |
190 | 'UNRESOLVED_BUGTASK_STATUSES', |
191 | + 'UNRESOLVED_PLUS_FIXRELEASED_BUGTASK_STATUSES', |
192 | 'UserCannotEditBugTaskImportance', |
193 | 'UserCannotEditBugTaskMilestone', |
194 | 'UserCannotEditBugTaskStatus', |
195 | @@ -287,6 +288,9 @@ |
196 | BugTaskStatus.INPROGRESS, |
197 | BugTaskStatus.FIXCOMMITTED) |
198 | |
199 | +UNRESOLVED_PLUS_FIXRELEASED_BUGTASK_STATUSES = ( |
200 | + UNRESOLVED_BUGTASK_STATUSES + (BugTaskStatus.FIXRELEASED,)) |
201 | + |
202 | RESOLVED_BUGTASK_STATUSES = ( |
203 | BugTaskStatus.FIXRELEASED, |
204 | BugTaskStatus.INVALID, |
205 | |
206 | === modified file 'lib/lp/bugs/model/bugtarget.py' |
207 | --- lib/lp/bugs/model/bugtarget.py 2010-01-21 16:47:24 +0000 |
208 | +++ lib/lp/bugs/model/bugtarget.py 2010-02-02 18:46:23 +0000 |
209 | @@ -1,4 +1,4 @@ |
210 | -# Copyright 2009 Canonical Ltd. This software is licensed under the |
211 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
212 | # GNU Affero General Public License version 3 (see the file LICENSE). |
213 | |
214 | # pylint: disable-msg=E0611,W0212 |
215 | |
216 | === added directory 'lib/lp/bugs/stories/patches-view' |
217 | === added file 'lib/lp/bugs/stories/patches-view/patches-view.txt' |
218 | --- lib/lp/bugs/stories/patches-view/patches-view.txt 1970-01-01 00:00:00 +0000 |
219 | +++ lib/lp/bugs/stories/patches-view/patches-view.txt 2010-02-02 18:46:23 +0000 |
220 | @@ -0,0 +1,323 @@ |
221 | +Patches View |
222 | +============ |
223 | + |
224 | +Patches View by Product |
225 | +----------------------- |
226 | + |
227 | +We have a view listing patches attached to bugs that target a given |
228 | +product. At first, the product is new and has no bugs. |
229 | + |
230 | + >>> patchy_product = factory.doAsUser( |
231 | + ... 'foo.bar@canonical.com', factory.makeProduct, |
232 | + ... name='patchy-product-1') |
233 | + |
234 | +We don't see any patches when we open the patches view. |
235 | + |
236 | + >>> def show_patches_view(contents): |
237 | + ... for tag in find_tags_by_class(contents, 'listing'): |
238 | + ... print extract_text(tag) |
239 | + |
240 | + >>> anon_browser.open( |
241 | + ... 'http://bugs.launchpad.dev/patchy-product-1/+patches') |
242 | + >>> show_patches_view(anon_browser.contents) |
243 | + Bug Importance Status Patch Age |
244 | + |
245 | +After the product has a bug, it still doesn't show up in the patches |
246 | +view, because that bug has no patch attachments. |
247 | + |
248 | + >>> from lp.bugs.interfaces.bugtask import ( |
249 | + ... BugTaskImportance, BugTaskStatus) |
250 | + >>> def make_bug( |
251 | + ... title, product, importance=BugTaskImportance.UNDECIDED, |
252 | + ... status=BugTaskStatus.NEW): |
253 | + ... bug = factory.makeBug(title=title, product=product) |
254 | + ... bug.default_bugtask.transitionToImportance( |
255 | + ... importance, product.owner) |
256 | + ... bug.default_bugtask.transitionToStatus( |
257 | + ... status, product.owner) |
258 | + ... return bug |
259 | + |
260 | + >>> bug_a = factory.doAsUser( |
261 | + ... 'foo.bar@canonical.com', make_bug, |
262 | + ... title="bug_a title", product=patchy_product) |
263 | + >>> anon_browser.open( |
264 | + ... 'http://bugs.launchpad.dev/patchy-product-1/+patches') |
265 | + >>> show_patches_view(anon_browser.contents) |
266 | + Bug Importance Status Patch Age |
267 | + |
268 | +After we add a non-patch attachment to that bug, the patches view |
269 | +still shows no patches. |
270 | + |
271 | + >>> factory.doAsUser('foo.bar@canonical.com', factory.makeBugAttachment, |
272 | + ... bug=bug_a, is_patch=False) |
273 | + <BugAttachment at... |
274 | + >>> anon_browser.open('http://bugs.launchpad.dev/patchy-product-1/+patches') |
275 | + >>> show_patches_view(anon_browser.contents) |
276 | + Bug Importance Status Patch Age |
277 | + |
278 | +After we add a patch attachment that's one day old, we see it in the |
279 | +patches view. |
280 | + |
281 | + >>> patch_submitter = factory.doAsUser( |
282 | + ... 'foo.bar@canonical.com', factory.makePerson, |
283 | + ... name="patchy-person", displayname="Patchy Person") |
284 | + >>> factory.doAsUser( |
285 | + ... 'foo.bar@canonical.com', factory.makeBugAttachment, |
286 | + ... comment="comment about patch a", |
287 | + ... filename="patch_a.diff", owner=patch_submitter, |
288 | + ... description="description of patch a", bug=bug_a, is_patch=True) |
289 | + <BugAttachment at... |
290 | + >>> anon_browser.open( |
291 | + ... 'http://bugs.launchpad.dev/patchy-product-1/+patches') |
292 | + >>> show_patches_view(anon_browser.contents) |
293 | + Bug Importance Status Patch Age |
294 | + Bug #16: bug_a title Undecided New ...second... |
295 | + From: Patchy Person |
296 | + Link: patch_a.diff description of patch a |
297 | + |
298 | +After creating some more bugs, with some non-patch and some patch |
299 | +attachments... |
300 | + |
301 | + >>> bug_b = factory.doAsUser( |
302 | + ... 'foo.bar@canonical.com', make_bug, |
303 | + ... title="bug_b title", product=patchy_product, |
304 | + ... importance=BugTaskImportance.CRITICAL, |
305 | + ... status=BugTaskStatus.CONFIRMED) |
306 | + >>> bug_c = factory.doAsUser( |
307 | + ... 'foo.bar@canonical.com', make_bug, |
308 | + ... title="bug_c title", product=patchy_product, |
309 | + ... importance=BugTaskImportance.WISHLIST, |
310 | + ... status=BugTaskStatus.FIXCOMMITTED) |
311 | + >>> factory.doAsUser( |
312 | + ... 'foo.bar@canonical.com', factory.makeBugAttachment, |
313 | + ... comment="comment about patch b", |
314 | + ... filename="patch_b.diff", owner=patch_submitter, |
315 | + ... description="description of patch b", bug=bug_b, is_patch=True) |
316 | + <BugAttachment at... |
317 | + >>> factory.doAsUser( |
318 | + ... 'foo.bar@canonical.com', factory.makeBugAttachment, |
319 | + ... comment="comment about patch c", |
320 | + ... filename="patch_c.diff", owner=patch_submitter, |
321 | + ... description="description of patch c", bug=bug_b, is_patch=True) |
322 | + <BugAttachment at... |
323 | + >>> factory.doAsUser( |
324 | + ... 'foo.bar@canonical.com', factory.makeBugAttachment, |
325 | + ... bug=bug_c, is_patch=False) |
326 | + <BugAttachment at... |
327 | + >>> factory.doAsUser( |
328 | + ... 'foo.bar@canonical.com', factory.makeBugAttachment, |
329 | + ... comment="comment about patch d", |
330 | + ... filename="patch_d.diff", owner=patch_submitter, |
331 | + ... description="description of patch d", bug=bug_c, is_patch=True) |
332 | + <BugAttachment at... |
333 | + >>> factory.doAsUser( |
334 | + ... 'foo.bar@canonical.com', factory.makeBugAttachment, |
335 | + ... comment="comment about patch e", |
336 | + ... filename="patch_e.diff", owner=patch_submitter, |
337 | + ... description="description of patch e", bug=bug_c, is_patch=True) |
338 | + <BugAttachment at... |
339 | + >>> factory.doAsUser( |
340 | + ... 'foo.bar@canonical.com', factory.makeBugAttachment, |
341 | + ... comment="comment about patch f", |
342 | + ... filename="patch_f.diff", owner=patch_submitter, |
343 | + ... description="description of patch f", bug=bug_c, is_patch=True) |
344 | + <BugAttachment at... |
345 | + |
346 | +...the youngest patch on each bug is visible is the patch report. |
347 | + |
348 | + >>> anon_browser.open('http://bugs.launchpad.dev/patchy-product-1/+patches') |
349 | + >>> show_patches_view(anon_browser.contents) |
350 | + Bug Importance Status Patch Age |
351 | + Bug #17: bug_b title Critical Confirmed ...second... |
352 | + From: Patchy Person |
353 | + Link: patch_c.diff description of patch c |
354 | + Bug #18: bug_c title Wishlist Fix Committed ...second... |
355 | + From: Patchy Person |
356 | + Link: patch_f.diff description of patch f |
357 | + Bug #16: bug_a title Undecided New ...second... |
358 | + From: Patchy Person |
359 | + Link: patch_a.diff description of patch a |
360 | + |
361 | +We can sort patches by importance and status. |
362 | + |
363 | + >>> anon_browser.getControl(name="orderby").value = ['-importance'] |
364 | + >>> anon_browser.getControl("sort").click() |
365 | + >>> anon_browser.url |
366 | + 'http://bugs.launchpad.dev/patchy-product-1/+patches?orderby=-importance' |
367 | + >>> show_patches_view(anon_browser.contents) |
368 | + Bug Importance Status Patch Age |
369 | + Bug #17: bug_b title Critical Confirmed ...second... |
370 | + From: Patchy Person |
371 | + Link: patch_c.diff description of patch c |
372 | + Bug #18: bug_c title Wishlist Fix Committed ...second... |
373 | + From: Patchy Person |
374 | + Link: patch_f.diff description of patch f |
375 | + Bug #16: bug_a title Undecided New ...second... |
376 | + From: Patchy Person |
377 | + Link: patch_a.diff description of patch a |
378 | + |
379 | + >>> anon_browser.getControl(name="orderby").value = ['status'] |
380 | + >>> anon_browser.getControl("sort").click() |
381 | + >>> anon_browser.url |
382 | + 'http://bugs.launchpad.dev/patchy-product-1/+patches?orderby=status' |
383 | + >>> show_patches_view(anon_browser.contents) |
384 | + Bug Importance Status Patch Age |
385 | + Bug #16: bug_a title Undecided New ...second... |
386 | + From: Patchy Person |
387 | + Link: patch_a.diff description of patch a |
388 | + Bug #17: bug_b title Critical Confirmed ...second... |
389 | + From: Patchy Person |
390 | + Link: patch_c.diff description of patch c |
391 | + Bug #18: bug_c title Wishlist Fix Committed ...second... |
392 | + From: Patchy Person |
393 | + Link: patch_f.diff description of patch f |
394 | + |
395 | +Bugs in a product series show up in the patches view for that series. |
396 | + |
397 | + >>> from zope.component import getUtility |
398 | + >>> from lp.registry.interfaces.distribution import IDistributionSet |
399 | + >>> def make_bugtask( |
400 | + ... # Meta-factory for making bugtasks. |
401 | + ... # |
402 | + ... # In all instances where a distro is needed, defaults to |
403 | + ... # 'ubuntu' distro. |
404 | + ... # |
405 | + ... # :param bug: The bug with which the task is associated. |
406 | + ... # :param target: The target to which to attach this bug. |
407 | + ... # If the target is a string, then it names the target |
408 | + ... # object, and exactly one of following two boolean |
409 | + ... # parameters must be set to indicate the object type. |
410 | + ... # :param target_is_spkg_name: If true, target is a string |
411 | + ... # indicating the name of the source package for the task. |
412 | + ... # :param target_is_distroseries_name: If true, target is a string |
413 | + ... # indicating the name of the distroseries for the task. |
414 | + ... # :param importance: The initial importance of the bugtask; |
415 | + ... # if None, just use the default importance. |
416 | + ... # :param status: The initial status of the bugtask; |
417 | + ... # if None, just use the default status. |
418 | + ... bug, target, |
419 | + ... target_is_spkg_name=False, |
420 | + ... target_is_distroseries_name=False, |
421 | + ... importance=None, status=None): |
422 | + ... ubuntu_distro = getUtility(IDistributionSet).getByName('ubuntu') |
423 | + ... if target_is_spkg_name: |
424 | + ... target = ubuntu_distro.getSourcePackage(target) |
425 | + ... if target_is_distroseries_name: |
426 | + ... target = ubuntu_distro.getSeries(target) |
427 | + ... bugtask = factory.makeBugTask(bug=bug, target=target) |
428 | + ... if importance is not None: |
429 | + ... bugtask.transitionToImportance(importance, ubuntu_distro.owner) |
430 | + ... if status is not None: |
431 | + ... bugtask.transitionToStatus(status, ubuntu_distro.owner) |
432 | + >>> patchy_product_series = patchy_product.getSeries('trunk') |
433 | + >>> factory.doAsUser( |
434 | + ... 'foo.bar@canonical.com', make_bugtask, |
435 | + ... bug=bug_a, target=patchy_product_series) |
436 | + >>> factory.doAsUser( |
437 | + ... 'foo.bar@canonical.com', make_bugtask, |
438 | + ... bug=bug_c, target=patchy_product_series) |
439 | + >>> anon_browser.open( |
440 | + ... 'https://launchpad.dev/patchy-product-1/trunk/+patches') |
441 | + >>> show_patches_view(anon_browser.contents) |
442 | + Bug Importance Status Patch Age |
443 | + Bug #18: bug_c title Wishlist Fix Committed ...second... |
444 | + From: Patchy Person |
445 | + Link: patch_f.diff |
446 | + description of patch f |
447 | + Bug #16: bug_a title Undecided New ...second... |
448 | + From: Patchy Person |
449 | + Link: patch_a.diff |
450 | + description of patch a |
451 | + |
452 | +Patches View by Distro |
453 | +---------------------- |
454 | + |
455 | +The patches view also works for distributions, and it shows the target |
456 | +package when viewed via a distribution. |
457 | + |
458 | + >>> factory.doAsUser( |
459 | + ... 'foo.bar@canonical.com', make_bugtask, bug=bug_a, |
460 | + ... target='evolution', target_is_spkg_name=True, |
461 | + ... importance=BugTaskImportance.MEDIUM, |
462 | + ... status=BugTaskStatus.FIXRELEASED) |
463 | + >>> factory.doAsUser( |
464 | + ... 'foo.bar@canonical.com', make_bugtask, bug=bug_c, |
465 | + ... target='a52dec', target_is_spkg_name=True, |
466 | + ... importance=BugTaskImportance.HIGH, |
467 | + ... status=BugTaskStatus.TRIAGED) |
468 | + |
469 | + >>> anon_browser.open('http://bugs.launchpad.dev/ubuntu/+patches') |
470 | + >>> show_patches_view(anon_browser.contents) |
471 | + Bug Importance Status Package Patch Age |
472 | + Bug #18: bug_c title High Triaged a52dec ...second... |
473 | + From: Patchy Person |
474 | + Link: patch_f.diff description of patch f |
475 | + Bug #16: bug_a title Medium Fix Released evolution ...second... |
476 | + From: Patchy Person |
477 | + Link: patch_a.diff description of patch a |
478 | + |
479 | +Patches View by Distro Series |
480 | +----------------------------- |
481 | + |
482 | +The patches view works for distro series. |
483 | + |
484 | + >>> factory.doAsUser( |
485 | + ... 'foo.bar@canonical.com', make_bugtask, bug=bug_a, |
486 | + ... target='hoary', target_is_distroseries_name=True) |
487 | + >>> factory.doAsUser( |
488 | + ... 'foo.bar@canonical.com', make_bugtask, bug=bug_a, |
489 | + ... target='warty', target_is_distroseries_name=True) |
490 | + >>> factory.doAsUser( |
491 | + ... 'foo.bar@canonical.com', make_bugtask, bug=bug_b, |
492 | + ... target='warty', target_is_distroseries_name=True) |
493 | + >>> factory.doAsUser( |
494 | + ... 'foo.bar@canonical.com', make_bugtask, bug=bug_c, |
495 | + ... target='warty', target_is_distroseries_name=True, |
496 | + ... importance=BugTaskImportance.HIGH, |
497 | + ... status=BugTaskStatus.TRIAGED) |
498 | + >>> anon_browser.open('https://launchpad.dev/ubuntu/hoary/+patches') |
499 | + >>> show_patches_view(anon_browser.contents) |
500 | + Bug Importance Status Package Patch Age |
501 | + Bug #16: bug_a title Undecided New hoary ...second... |
502 | + From: Patchy Person |
503 | + Link: patch_a.diff |
504 | + description of patch a |
505 | + >>> anon_browser.open('https://launchpad.dev/ubuntu/warty/+patches') |
506 | + >>> show_patches_view(anon_browser.contents) |
507 | + Bug Importance Status Package Patch Age |
508 | + Bug #18: bug_c title High Triaged warty ...second... |
509 | + From: Patchy Person |
510 | + Link: patch_f.diff description of patch f |
511 | + Bug #16: bug_a title Undecided New warty ...second... |
512 | + From: Patchy Person |
513 | + Link: patch_a.diff |
514 | + description of patch a |
515 | + Bug #17: bug_b title Undecided New warty ...second... |
516 | + From: Patchy Person |
517 | + Link: patch_c.diff |
518 | + description of patch c |
519 | + |
520 | +Patches View by Source Package |
521 | +------------------------------ |
522 | + |
523 | +The patches view works for source packages too. The view doesn't show |
524 | +target package column in that case, because the package is implied. |
525 | + |
526 | + >>> anon_browser.open( |
527 | + ... 'http://bugs.launchpad.dev/ubuntu/+source/a52dec/+patches') |
528 | + >>> show_patches_view(anon_browser.contents) |
529 | + Bug Importance Status Patch Age |
530 | + Bug #18: bug_c title High Triaged ...second... |
531 | + From: Patchy Person |
532 | + Link: patch_f.diff description of patch f |
533 | + >>> anon_browser.open( |
534 | + ... 'http://bugs.launchpad.dev/ubuntu/+source/evolution/+patches') |
535 | + >>> show_patches_view(anon_browser.contents) |
536 | + Bug Importance Status Patch Age |
537 | + Bug #16: bug_a title Medium Fix Released ...second... |
538 | + From: Patchy Person |
539 | + Link: patch_a.diff description of patch a |
540 | + |
541 | +# XXX Karl Fogel 2010-02-02 bug=516186: We should also test the |
542 | +# patches view on a project group, to make sure the "Package" column |
543 | +# becomes "Project" in that context. |
544 | |
545 | === added file 'lib/lp/bugs/templates/bugtarget-patches.pt' |
546 | --- lib/lp/bugs/templates/bugtarget-patches.pt 1970-01-01 00:00:00 +0000 |
547 | +++ lib/lp/bugs/templates/bugtarget-patches.pt 2010-02-02 18:46:23 +0000 |
548 | @@ -0,0 +1,125 @@ |
549 | +<html |
550 | + xmlns="http://www.w3.org/1999/xhtml" |
551 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
552 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
553 | + xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
554 | + xml:lang="en" |
555 | + lang="en" |
556 | + dir="ltr" |
557 | + metal:use-macro="view/macro:page/main_only" |
558 | + i18n:domain="malone" |
559 | +> |
560 | + |
561 | + <body> |
562 | + <div metal:fill-slot="main" class="tab-bugs" |
563 | + tal:define="batchnav view/batchedPatchTasks; |
564 | + batch batchnav/currentBatch"> |
565 | + |
566 | + <form class="lesser" id="sort" method="get" |
567 | + tal:attributes="action string:${context/fmt:url}/+patches"> |
568 | + |
569 | + <script type="text/javascript"> |
570 | + YUI().use('base', 'node', 'event', function(Y) { |
571 | + Y.on('domready', function(e) { |
572 | + Y.get('#sort-button').setStyle('display', 'none'); |
573 | + Y.get('#orderby').on('change', function(e) { |
574 | + Y.get('#sort').submit(); |
575 | + }); |
576 | + }); |
577 | + }); |
578 | + </script> |
579 | + |
580 | + Order by: <select |
581 | + name="orderby" id="orderby" size="1" |
582 | + tal:define="orderby request/orderby|string:-importance"> |
583 | + <option |
584 | + value="-importance" |
585 | + tal:attributes="selected python:orderby == '-importance'" |
586 | + >importance</option> |
587 | + <option |
588 | + value="status" |
589 | + tal:attributes="selected python:orderby == 'status'" |
590 | + >status</option> |
591 | + <option |
592 | + tal:condition="view/targetName" |
593 | + tal:content="view/targetName" |
594 | + tal:attributes="selected python:orderby == 'targetname'" |
595 | + value="targetname" |
596 | + ></option> |
597 | + <option |
598 | + value="datecreated" |
599 | + tal:attributes="selected python:orderby == 'datecreated'" |
600 | + >oldest first</option> |
601 | + <option |
602 | + value="-datecreated" |
603 | + tal:attributes="selected python:orderby == '-datecreated'" |
604 | + >newest first</option> |
605 | + </select> |
606 | + <input type="submit" value="sort" id="sort-button"/> |
607 | + </form> |
608 | + |
609 | + <table class="listing"><thead> |
610 | + <tr> |
611 | + <th class="patches-view-cell" |
612 | + >Bug</th> |
613 | + <th class="patches-view-cell" |
614 | + >Importance</th> |
615 | + <th class="patches-view-cell" |
616 | + >Status</th> |
617 | + <th class="patches-view-cell" |
618 | + tal:condition="view/targetName" |
619 | + tal:content="view/targetName" |
620 | + ></th> |
621 | + <th class="patches-view-cell" |
622 | + >Patch Age</th> |
623 | + </tr> |
624 | + </thead> |
625 | + <tr tal:repeat="patch_task batch"> |
626 | + <td class="patches-view-cell"> |
627 | + <a tal:replace="structure patch_task/fmt:link" /></td> |
628 | + <td class="patches-view-cell" |
629 | + tal:content="patch_task/importance/title" |
630 | + tal:attributes="class string:importance${patch_task/importance/name}"></td> |
631 | + <td class="patches-view-cell" |
632 | + tal:content="patch_task/status/title" |
633 | + tal:attributes="class string:status${patch_task/status/name}"></td> |
634 | + <td class="patches-view-cell" |
635 | + tal:condition="view/targetName"> |
636 | + <a tal:attributes="href patch_task/target/fmt:url" |
637 | + tal:content="patch_task/target/name"></a></td> |
638 | + <td class="patches-view-cell" |
639 | + tal:define="p python:view.youngestPatch(patch_task.bug); |
640 | + age python:view.patchAge(p)" |
641 | + tal:attributes="id string:patch-cell-${repeat/patch_task/index}"> |
642 | + <a tal:attributes="href p/libraryfile/http_url" |
643 | + tal:content="age/fmt:approximateduration/use-digits"></a> |
644 | + <div class="popupTitle" |
645 | + tal:attributes="id string:patch-popup-${repeat/patch_task/index};"> |
646 | + <p tal:define="submitter p/message/owner"> |
647 | + <strong>From:</strong> |
648 | + <a tal:replace="structure submitter/fmt:link"></a><br/> |
649 | + <strong>Link:</strong> |
650 | + <a tal:attributes="href p/libraryfile/http_url" |
651 | + tal:content="p/libraryfile/filename"></a></p> |
652 | + <p tal:content="string:${p/title}"></p> |
653 | + </div> |
654 | + <script type="text/javascript" tal:content="string: |
655 | + YUI().use('base', 'node', 'event', function(Y) { |
656 | + Y.on('domready', function(e) { |
657 | + var cell_id = '#patch-cell-${repeat/patch_task/index}'; |
658 | + var target_id = '#patch-popup-${repeat/patch_task/index}'; |
659 | + var elt = Y.get(cell_id); |
660 | + elt.on('mouseover', function(e) { |
661 | + Y.get(target_id).setStyle('display', 'block'); |
662 | + }); |
663 | + elt.on('mouseout', function(e) { |
664 | + Y.get(target_id).setStyle('display', 'none'); |
665 | + }); |
666 | + }); |
667 | + });"/> |
668 | + </td> |
669 | + </tr></table> |
670 | + <div tal:replace="structure batchnav/@@+navigation-links-lower" /> |
671 | + </div><!-- main --> |
672 | + </body> |
673 | +</html> |
674 | |
675 | === modified file 'lib/lp/testing/factory.py' |
676 | --- lib/lp/testing/factory.py 2010-01-30 05:27:48 +0000 |
677 | +++ lib/lp/testing/factory.py 2010-02-02 18:46:23 +0000 |
678 | @@ -130,7 +130,7 @@ |
679 | from lp.soyuz.interfaces.component import IComponentSet |
680 | from lp.soyuz.interfaces.packageset import IPackagesetSet |
681 | from lp.soyuz.model.buildqueue import BuildQueue |
682 | -from lp.testing import run_with_login, time_counter |
683 | +from lp.testing import run_with_login, time_counter, login, logout |
684 | |
685 | SPACE = ' ' |
686 | |
687 | @@ -239,6 +239,21 @@ |
688 | %s |
689 | ''') |
690 | |
691 | + def doAsUser(self, user, factory_method, **factory_args): |
692 | + """Perform a factory method while temporarily logged in as a user. |
693 | + |
694 | + :param user: The user to log in as, and then to log out from. |
695 | + :param factory_method: The factory method to invoke while logged in. |
696 | + :param factory_args: Keyword arguments to pass to factory_method. |
697 | + """ |
698 | + login(user) |
699 | + try: |
700 | + result = factory_method(**factory_args) |
701 | + transaction.commit() |
702 | + finally: |
703 | + logout() |
704 | + return result |
705 | + |
706 | def makeCopyArchiveLocation(self, distribution=None, owner=None, |
707 | name=None, enabled=True): |
708 | """Create and return a new arbitrary location for copy packages.""" |
709 | @@ -1084,7 +1099,7 @@ |
710 | |
711 | def makeBugAttachment(self, bug=None, owner=None, data=None, |
712 | comment=None, filename=None, content_type=None, |
713 | - description=None): |
714 | + description=None, is_patch=_DEFAULT): |
715 | """Create and return a new bug attachment. |
716 | |
717 | :param bug: An `IBug` or a bug ID or name, or None, in which |
718 | @@ -1099,6 +1114,7 @@ |
719 | string will be used. |
720 | :param content_type: The MIME-type of this file. |
721 | :param description: The description of the attachment. |
722 | + :param is_patch: If true, this attachment is a patch. |
723 | :return: An `IBugAttachment`. |
724 | """ |
725 | if bug is None: |
726 | @@ -1115,9 +1131,16 @@ |
727 | comment = self.getUniqueString() |
728 | if filename is None: |
729 | filename = self.getUniqueString() |
730 | + # If the default value of is_patch when creating a new |
731 | + # BugAttachment should ever change, we don't want to interfere |
732 | + # with that. So, we only override it if our caller explicitly |
733 | + # passed it. |
734 | + other_params = {} |
735 | + if is_patch is not _DEFAULT: |
736 | + other_params['is_patch'] = is_patch |
737 | return bug.addAttachment( |
738 | owner, data, comment, filename, content_type=content_type, |
739 | - description=description) |
740 | + description=description, **other_params) |
741 | |
742 | def makeSignedMessage(self, msgid=None, body=None, subject=None, |
743 | attachment_contents=None, force_transfer_encoding=False, |
With Abel, add a "+patches" view based on IHasBugs.
The patch report lists bugtasks -- for bugs that have patch attachments -- on products, series, and packages. The listing can be sorted by importance, status, and bug age. (Sorting on patch age is not yet implemented, and will be done a separate branch. Likewise, the patch report for persons/teams is not yet implemented, and will be a separate branch.)
To review, load new sampledata from http:// people. canonical. com/~kfogel/ 506018/ newsampledata- dev.sql. gz . Then visit pages like these:
https:/ /bugs.launchpad .dev/patches- view-test/ +patches /bugs.launchpad .dev/firefox/ +patches /launchpad. dev/ubuntu/ +patches /bugs.launchpad .dev/ubuntu/ +source/ cdrkit/ +patches
https:/
https:/
https:/