Merge into devel : show-oops-ids-bug-564686 : Code : Launchpad itself

Status:

Merged

Approved by:

Aaron Bentley on 2010-05-17

Approved revision:

no longer in the source branch.

Merged at revision:

10880

Proposed branch:

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

Merge into:

lp://staging/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://staging/~gmb/launchpad/show-oops-ids-bug-564686

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Aaron Bentley (community)		2010-05-17	Approve on 2010-05-17
Review via email: mp+25442@code.staging.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.

Revision history for this message

Aaron Bentley (abentley) wrote on 2010-05-17:

#

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 on 2010-05-17:

#

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_...

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_string))
+
+
 def test_suite():
     """Return this module's doctest Suite. Unit tests are also run."""
     suite = unittest.TestSuite()

=== modified file 'lib/lp/bugs/browser/bugwatch.py'
--- lib/lp/bugs/browser/bugwatch.py	2010-05-17 15:21:47 +0000
+++ lib/lp/bugs/browser/bugwatch.py	2010-05-17 15:54:18 +0000
@@ -180,7 +180,6 @@
         """Return a list of dicts representing recent watch activity."""
         activity_items = []
         for activity in self.context.activity:
-            oops_str = None
             if activity.result in BUG_WATCH_ACTIVITY_SUCCESS_STATUSES:
                 icon = "/@@/yes"
                 completion_message = "completed successfully"
@@ -189,24 +188,12 @@
                 completion_message = (
                     "failed with error '%s'" % activity.result.title)
 
-                if activity.oops_id is not None:
-                    if getUtility(ILaunchBag).developer:
-                        # If the user is an LP developer we linkify the
-                        # OOPS ID.
-                        root_url = config.launchpad.oops_root_url
-                        oops_url = root_url + activity.oops_id
-                        oops_str = '<a href="%s">%s</a>' % (
-                            oops_url, activity.oops_id)
-                    else:
-                        oops_str = activity.oops_id
-
-
             activity_items.append({
                 'icon': icon,
                 'date': activity.activity_date,
                 'completion_message': completion_message,
                 'result_text': activity.result.title,
-                'oops_str': oops_str,
+                'oops_id': activity.oops_id,
                 })
 
         return activity_items

=== modified file 'lib/lp/bugs/templates/bugwatch-portlet-activity.pt'
--- lib/lp/bugs/templates/bugwatch-portlet-activity.pt	2010-05-17 15:21:47 +0000
+++ lib/lp/bugs/templates/bugwatch-portlet-activity.pt	2010-05-17 15:54:18 +0000
@@ -37,8 +37,8 @@
         <img tal:attributes="src activity/icon; title activity/result_text" />
         Update
         <tal:message replace="activity/completion_message" />
-        <tal:oopsid condition="activity/oops_str">
-          (<tal:oops_link replace="structure activity/oops_str" />)
+        <tal:oopsid condition="activity/oops_id">
+          (<tal:oops_link replace="structure activity/oops_id/fmt:oops-id" />)
         </tal:oopsid>
         <tal:time replace="activity/date/fmt:displaydate" />
       </div>

Revision history for this message

Aaron Bentley (abentley) wrote on 2010-05-17:

#

Looks good.

review: Approve

Launchpad itself

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

Commit message

Description of the change

Preview Diff

Subscribers