Merge lp:~gmb/launchpad/show-oops-ids-bug-564686 into lp:launchpad

Proposed by Graham Binns
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: 10880
Proposed branch: lp:~gmb/launchpad/show-oops-ids-bug-564686
Merge into: lp:launchpad
Diff against target: 221 lines (+95/-9)
6 files modified
lib/canonical/launchpad/webapp/tales.py (+12/-0)
lib/canonical/launchpad/webapp/tests/test_tales.py (+41/-0)
lib/lp/bugs/browser/bugwatch.py (+2/-0)
lib/lp/bugs/browser/tests/bugwatch-views.txt (+3/-0)
lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt (+34/-9)
lib/lp/bugs/templates/bugwatch-portlet-activity.pt (+3/-0)
To merge this branch: bzr merge lp:~gmb/launchpad/show-oops-ids-bug-564686
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+25442@code.launchpad.net

Commit message

OOPS IDs for bug watch update failures will now be shown on the watches' +edit pages.

Description of the change

This branch adds the OOPS IDs to the error messages shown on the bug watch +edit pages for bug watch updates that have failed.

I've updated the recent_watch_activity property of BugWatchActivityPortletView to include the OOPS ID where appropriate. If the user is a Launchpad developer, the OOPS ID will be linkified.

I've added tests to the xx-edit-bugwatch.txt story to cover this. I've also updated the existing tests to use user_browser rather than admin browser, since this should be visible to all users, not just admins.

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

It would be good if you could generalize the OOPs URL generation so that we're using the same codepath everywhere, but this is not strictly required to land this.

review: Approve
Revision history for this message
Graham Binns (gmb) wrote :
Download full text (5.6 KiB)

I've added an oops-id formatter and tests to go with it. Here's a diff of the new code:

=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py 2010-05-05 20:01:56 +0000
+++ lib/canonical/launchpad/webapp/tales.py 2010-05-17 15:54:18 +0000
@@ -3025,6 +3025,16 @@
         else:
             return id_

+ def oops_id(self):
+ """Format an OOPS ID for display."""
+ if not getUtility(ILaunchBag).developer:
+ # We only linkify OOPS IDs for Launchpad developers.
+ return self._stringtoformat
+
+ root_url = config.launchpad.oops_root_url
+ url = root_url + self._stringtoformat
+ return '<a href="%s">%s</a>' % (url, self._stringtoformat)
+
     def traverse(self, name, furtherPath):
         if name == 'nl_to_br':
             return self.nl_to_br()
@@ -3063,6 +3073,8 @@
                 return self.css_id(furtherPath.pop())
             else:
                 return self.css_id()
+ elif name == 'oops-id':
+ return self.oops_id()
         else:
             raise TraversalError(name)

=== modified file 'lib/canonical/launchpad/webapp/tests/test_tales.py'
--- lib/canonical/launchpad/webapp/tests/test_tales.py 2009-11-18 00:22:41 +0000
+++ lib/canonical/launchpad/webapp/tests/test_tales.py 2010-05-17 16:43:54 +0000
@@ -6,9 +6,12 @@
 from textwrap import dedent
 import unittest

+from zope.component import getUtility
 from zope.testing.doctestunit import DocTestSuite

+from canonical.config import config
 from canonical.launchpad.testing.pages import find_tags_by_class
+from canonical.launchpad.webapp.interfaces import ILaunchBag
 from canonical.launchpad.webapp.tales import FormattersAPI
 from canonical.testing import DatabaseFunctionalLayer
 from lp.testing import TestCase
@@ -263,6 +266,44 @@
         self.assertEqual(3, line_count)

+class TestOOPSFormatter(TestCase):
+ """A test case for the oops_id() string formatter."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_doesnt_linkify_for_non_developers(self):
+ # OOPS IDs won't be linkified for non-developers.
+ oops_id = 'OOPS-12345TEST'
+ formatter = FormattersAPI(oops_id)
+ formatted_string = formatter.oops_id()
+
+ self.assertEqual(
+ oops_id, formatted_string,
+ "Formatted string should be '%s', was '%s'" % (
+ oops_id, formatted_string))
+
+ def _setDeveloper(self):
+ """Override ILaunchBag.developer for testing purposes."""
+ launch_bag = getUtility(ILaunchBag)
+ launch_bag.setDeveloper(True)
+
+ def test_linkifies_for_developers(self):
+ # OOPS IDs will be linkified for Launchpad developers.
+ oops_id = 'OOPS-12345TEST'
+ formatter = FormattersAPI(oops_id)
+
+ self._setDeveloper()
+ formatted_string = formatter.oops_id()
+
+ expected_string = '<a href="%s">%s</a>' % (
+ config.launchpad.oops_root_url + oops_id, oops_id)
+
+ self.assertEqual(
+ expected_string, formatted_string,
+ "Formatted string should be '%s', was '%s'" % (
+ expected_string, formatted_...

Read more...

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

Looks good.

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/webapp/tales.py'
2--- lib/canonical/launchpad/webapp/tales.py 2010-05-05 20:01:56 +0000
3+++ lib/canonical/launchpad/webapp/tales.py 2010-05-18 10:29:29 +0000
4@@ -3025,6 +3025,16 @@
5 else:
6 return id_
7
8+ def oops_id(self):
9+ """Format an OOPS ID for display."""
10+ if not getUtility(ILaunchBag).developer:
11+ # We only linkify OOPS IDs for Launchpad developers.
12+ return self._stringtoformat
13+
14+ root_url = config.launchpad.oops_root_url
15+ url = root_url + self._stringtoformat
16+ return '<a href="%s">%s</a>' % (url, self._stringtoformat)
17+
18 def traverse(self, name, furtherPath):
19 if name == 'nl_to_br':
20 return self.nl_to_br()
21@@ -3063,6 +3073,8 @@
22 return self.css_id(furtherPath.pop())
23 else:
24 return self.css_id()
25+ elif name == 'oops-id':
26+ return self.oops_id()
27 else:
28 raise TraversalError(name)
29
30
31=== modified file 'lib/canonical/launchpad/webapp/tests/test_tales.py'
32--- lib/canonical/launchpad/webapp/tests/test_tales.py 2009-11-18 00:22:41 +0000
33+++ lib/canonical/launchpad/webapp/tests/test_tales.py 2010-05-18 10:29:29 +0000
34@@ -6,9 +6,12 @@
35 from textwrap import dedent
36 import unittest
37
38+from zope.component import getUtility
39 from zope.testing.doctestunit import DocTestSuite
40
41+from canonical.config import config
42 from canonical.launchpad.testing.pages import find_tags_by_class
43+from canonical.launchpad.webapp.interfaces import ILaunchBag
44 from canonical.launchpad.webapp.tales import FormattersAPI
45 from canonical.testing import DatabaseFunctionalLayer
46 from lp.testing import TestCase
47@@ -263,6 +266,44 @@
48 self.assertEqual(3, line_count)
49
50
51+class TestOOPSFormatter(TestCase):
52+ """A test case for the oops_id() string formatter."""
53+
54+ layer = DatabaseFunctionalLayer
55+
56+ def test_doesnt_linkify_for_non_developers(self):
57+ # OOPS IDs won't be linkified for non-developers.
58+ oops_id = 'OOPS-12345TEST'
59+ formatter = FormattersAPI(oops_id)
60+ formatted_string = formatter.oops_id()
61+
62+ self.assertEqual(
63+ oops_id, formatted_string,
64+ "Formatted string should be '%s', was '%s'" % (
65+ oops_id, formatted_string))
66+
67+ def _setDeveloper(self):
68+ """Override ILaunchBag.developer for testing purposes."""
69+ launch_bag = getUtility(ILaunchBag)
70+ launch_bag.setDeveloper(True)
71+
72+ def test_linkifies_for_developers(self):
73+ # OOPS IDs will be linkified for Launchpad developers.
74+ oops_id = 'OOPS-12345TEST'
75+ formatter = FormattersAPI(oops_id)
76+
77+ self._setDeveloper()
78+ formatted_string = formatter.oops_id()
79+
80+ expected_string = '<a href="%s">%s</a>' % (
81+ config.launchpad.oops_root_url + oops_id, oops_id)
82+
83+ self.assertEqual(
84+ expected_string, formatted_string,
85+ "Formatted string should be '%s', was '%s'" % (
86+ expected_string, formatted_string))
87+
88+
89 def test_suite():
90 """Return this module's doctest Suite. Unit tests are also run."""
91 suite = unittest.TestSuite()
92
93=== modified file 'lib/lp/bugs/browser/bugwatch.py'
94--- lib/lp/bugs/browser/bugwatch.py 2010-04-20 20:28:59 +0000
95+++ lib/lp/bugs/browser/bugwatch.py 2010-05-18 10:29:29 +0000
96@@ -14,6 +14,7 @@
97 from zope.component import getUtility
98 from zope.interface import Interface
99
100+from canonical.config import config
101 from canonical.database.constants import UTC_NOW
102 from canonical.widgets.textwidgets import URIWidget
103
104@@ -192,6 +193,7 @@
105 'date': activity.activity_date,
106 'completion_message': completion_message,
107 'result_text': activity.result.title,
108+ 'oops_id': activity.oops_id,
109 })
110
111 return activity_items
112
113=== modified file 'lib/lp/bugs/browser/tests/bugwatch-views.txt'
114--- lib/lp/bugs/browser/tests/bugwatch-views.txt 2010-04-19 15:49:11 +0000
115+++ lib/lp/bugs/browser/tests/bugwatch-views.txt 2010-05-18 10:29:29 +0000
116@@ -84,6 +84,7 @@
117 {'completion_message': 'completed successfully',
118 'date': datetime.datetime(...tzinfo=<UTC>),
119 'icon': '/@@/yes',
120+ 'oops_id': None,
121 'result_text': 'Synchronisation succeeded'}
122
123 If an activity entry records a failure, the 'icon' entry in the dict
124@@ -103,10 +104,12 @@
125 {'completion_message': "failed with error 'Bug Not Found'",
126 'date': datetime.datetime(...tzinfo=<UTC>),
127 'icon': '/@@/no',
128+ 'oops_id': None,
129 'result_text': 'Bug Not Found'}
130 {'completion_message': 'completed successfully',
131 'date': datetime.datetime(...tzinfo=<UTC>),
132 'icon': '/@@/yes',
133+ 'oops_id': None,
134 'result_text': 'Synchronisation succeeded'}
135
136
137
138=== modified file 'lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt'
139--- lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt 2010-05-13 12:49:15 +0000
140+++ lib/lp/bugs/stories/bugwatches/xx-edit-bugwatch.txt 2010-05-18 10:29:29 +0000
141@@ -86,9 +86,9 @@
142 activity entries. When a watch has not been checked, no activity is
143 shown.
144
145- >>> admin_browser.open('http://bugs.launchpad.dev/bugs/1/+watch/2')
146+ >>> user_browser.open('http://bugs.launchpad.dev/bugs/1/+watch/2')
147 >>> recent_activity_list = find_tag_by_id(
148- ... admin_browser.contents, 'recent-watch-activity')
149+ ... user_browser.contents, 'recent-watch-activity')
150 >>> print recent_activity_list
151 None
152
153@@ -100,9 +100,9 @@
154 >>> watch.addActivity()
155 >>> logout()
156
157- >>> admin_browser.open('http://bugs.launchpad.dev/bugs/1/+watch/2')
158+ >>> user_browser.open('http://bugs.launchpad.dev/bugs/1/+watch/2')
159 >>> recent_activity_list = find_tag_by_id(
160- ... admin_browser.contents, 'recent-watch-activity')
161+ ... user_browser.contents, 'recent-watch-activity')
162 >>> print extract_text(recent_activity_list)
163 Update completed successfully ... ago
164
165@@ -114,12 +114,37 @@
166 >>> watch.addActivity(result=BugWatchActivityStatus.BUG_NOT_FOUND)
167 >>> logout()
168
169+ >>> user_browser.open('http://bugs.launchpad.dev/bugs/1/+watch/2')
170+ >>> recent_activity_list = find_tag_by_id(
171+ ... user_browser.contents, 'recent-watch-activity')
172+ >>> print extract_text(recent_activity_list)
173+ Update failed with error 'Bug Not Found' ... ago
174+ Update completed successfully ... ago
175+
176+If a failure has an OOPS ID attached to it, that too will be reflected
177+in the list.
178+
179+ >>> login('foo.bar@canonical.com')
180+ >>> watch = getUtility(IBugWatchSet).get(2)
181+ >>> watch.addActivity(
182+ ... result=BugWatchActivityStatus.COMMENT_IMPORT_FAILED,
183+ ... oops_id="OOPS-12345TEST")
184+ >>> logout()
185+
186+ >>> user_browser.open('http://bugs.launchpad.dev/bugs/1/+watch/2')
187+ >>> recent_activity_list = find_tag_by_id(
188+ ... user_browser.contents, 'recent-watch-activity')
189+ >>> print extract_text(recent_activity_list)
190+ Update failed with error 'Unable to import...' (OOPS-12345TEST) ... ago
191+ Update failed with error 'Bug Not Found' ... ago
192+ Update completed successfully ... ago
193+
194+If a Launchpad developer views the page the OOPS IDs will be linkified.
195+
196 >>> admin_browser.open('http://bugs.launchpad.dev/bugs/1/+watch/2')
197- >>> recent_activity_list = find_tag_by_id(
198- ... admin_browser.contents, 'recent-watch-activity')
199- >>> print extract_text(recent_activity_list)
200- Update failed with error 'Bug Not Found' ... ago
201- Update completed successfully ... ago
202+ >>> oops_link = admin_browser.getLink('OOPS-12345TEST')
203+ >>> print oops_link.url
204+ http...OOPS-12345TEST
205
206
207 Rescheduling a watch
208
209=== modified file 'lib/lp/bugs/templates/bugwatch-portlet-activity.pt'
210--- lib/lp/bugs/templates/bugwatch-portlet-activity.pt 2010-04-19 19:26:05 +0000
211+++ lib/lp/bugs/templates/bugwatch-portlet-activity.pt 2010-05-18 10:29:29 +0000
212@@ -37,6 +37,9 @@
213 <img tal:attributes="src activity/icon; title activity/result_text" />
214 Update
215 <tal:message replace="activity/completion_message" />
216+ <tal:oopsid condition="activity/oops_id">
217+ (<tal:oops_link replace="structure activity/oops_id/fmt:oops-id" />)
218+ </tal:oopsid>
219 <tal:time replace="activity/date/fmt:displaydate" />
220 </div>
221 </tal:activity>