Merge lp:~kfogel/launchpad/506018-patch-report into lp:launchpad

Proposed by Karl Fogel
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
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
To post a comment you must log in.
Revision history for this message
Karl Fogel (kfogel) wrote :

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
  https://bugs.launchpad.dev/firefox/+patches
  https://launchpad.dev/ubuntu/+patches
  https://bugs.launchpad.dev/ubuntu/+source/cdrkit/+patches

Revision history for this message
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

review: Needs Fixing (ui)
Revision history for this message
Eleanor Berger (intellectronica) wrote :
Download full text (30.7 KiB)

> === modified file 'lib/canonical/launchpad/icing/style.css'
> --- lib/canonical/launchpad/icing/style.css     2010-01-20 21:57:44 +0000
> +++ lib/canonical/launchpad/icing/style.css     2010-01-28 17:24:20 +0000
> @@ -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/bugs/browser/bugtarget.py'
> --- lib/lp/bugs/browser/bugtarget.py    2010-01-07 05:41:58 +0000
> +++ lib/lp/bugs/browser/bugtarget.py    2010-01-28 17:24:20 +0000
> @@ -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-related browser views."""
> @@ -15,12 +15,15 @@
>     "FileBugViewBase",
>     "OfficialBugTagsManageView",
>     "ProjectFileBugGuidedView",
> +    "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.browser.bugtask import BugTaskSearchListingView
>  from lp.bugs.interfaces.bug import IBug
> +from lp.bugs.interfaces.bugattachment import BugAttachmentType
>  from canonical.launchpad.browser.feeds import (
>     BugFeedLink, BugTargetLatestBugsFeedLink, FeedsMixin,
>     PersonLatestBugsFeedLink)
> @@ -73,6 +77,7 @@
>     LaunchpadEditFormView, LaunchpadFormView, LaunchpadView, action,
>     canonical_url, custom_widget, safe_action)
>  from canonical.launchpad.webapp.authorization import check_permission
> +from canonical.launchpad.webapp.batching import BatchNavigator
>  from canonical.launchpad.webapp.tales import BugTrackerFormatterAPI
>  from canonical.launchpad.validators.name import valid_name_pattern
>  from canonical.launchpad.webapp.menu import structured
> @@ -1372,3 +1377,51 @@
>  class BugsVHostBreadcrumb(Breadcrumb):
>     rootsite = 'bugs'
>     text = 'Bugs'
> +
> +
> +class BugsPatchesView(LaunchpadView):
> +    """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.displayname
> +
> +    def batchedPatchTasks(self):
> +        """Return a BatchNavigator for bug tasks with patch attachments."""
> +        any_unresolved_plus_fixreleased = \
> +            UNRESOLVED_BUGTASK_STATUSES + (BugTaskStatus.FIXRELEASED,)

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:

       any_unresolved_plus_fixreleased = (
           UNRESOLVED_BUGTASK_STATUSES + (BugTaskStatus.FIXR...

review: Needs Fixing
Revision history for this message
Abel Deuring (adeuring) wrote :

On 28.01.2010 19:23, Tom Berger wrote:
>> === added directory 'lib/lp/bugs/stories/patches-view'
>> === added file 'lib/lp/bugs/stories/patches-view/patches-view.txt'
>> --- lib/lp/bugs/stories/patches-view/patches-view.txt 1970-01-01 00:00:00 +0000
>> +++ lib/lp/bugs/stories/patches-view/patches-view.txt 2010-01-28 17:24:20 +0000
>> @@ -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(factory_method, **thing_args):
>> + ... login('<email address hidden>')
>> + ... result = factory_method(**thing_args)
>> + ... transaction.commit()
>> + ... 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.database_login('<email address hidden>'):
 factory.makePerson(k1=v1, k2=v2)

would look much nicer than

    make_thing(factory.makePerson, k1=v1, k2=v2...)

Abel

Revision history for this message
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/bugs/stories/patches-view'
>>> === added file 'lib/lp/bugs/stories/patches-view/patches-view.txt'
>>> --- lib/lp/bugs/stories/patches-view/patches-view.txt   1970-01-01 00:00:00 +0000
>>> +++ lib/lp/bugs/stories/patches-view/patches-view.txt   2010-01-28 17:24:20 +0000
>>> @@ -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(factory_method, **thing_args):
>>> +    ...     login('<email address hidden>')
>>> +    ...     result = factory_method(**thing_args)
>>> +    ...     transaction.commit()
>>> +    ...     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.database_login('<email address hidden>'):
>        factory.makePerson(k1=v1, k2=v2)
>
> would look much nicer than
>
>    make_thing(factory.makePerson, k1=v1, k2=v2...)

+1000000

Revision history for this message
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.

Revision history for this message
Karl Fogel (kfogel) wrote :

Note that Michael Hudson effectively reviewed my re-submission when he reviewed

 https://code.edge.launchpad.net/~intellectronica/launchpad/no-patches-message/+merge/18428

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.

review: Needs Fixing
Revision history for this message
Eleanor Berger (intellectronica) wrote :

You've addressed all of my concerns and the code looks great now.

review: Approve (code)
Revision history for this message
Abel Deuring (adeuring) wrote :

Hi Karl,

I looked at the changes between revision 10189 and 10194 -- everything is fine.

review: Approve (code)
Revision history for this message
Abel Deuring (adeuring) wrote :
Revision history for this message
Martin Albisetti (beuno) wrote :

A wonderful piece of work. The sooner this lands, the better.

review: Approve (ui)
Revision history for this message
Karl Fogel (kfogel) wrote :

LGTM too now, not that I'm a reviewer, and it's mostly my own change.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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&nbsp;by:&nbsp;<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,