launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/link-recipe-ppa-tal into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/link-recipe-ppa-tal/+merge/51698

This branch massively refactors the recent work done with displaying recipe details on the +packages page for a PPA. It moves the entire logic for the string into the view, and makes use of structured text to handle escaping the arguments. I also expanded structured itself to not escape arguments that are instances of structured itself.

To test: bin/test -vvt TestSourcePublicationListingExtra
-- 
https://code.launchpad.net/~stevenk/launchpad/link-recipe-ppa-tal/+merge/51698
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/link-recipe-ppa-tal into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/menu.py'
--- lib/canonical/launchpad/webapp/menu.py	2010-10-12 13:29:11 +0000
+++ lib/canonical/launchpad/webapp/menu.py	2011-03-01 05:06:15 +0000
@@ -72,13 +72,21 @@
                 "You must provide either positional arguments or keyword "
                 "arguments to structured(), not both.")
         if replacements:
-            self.escapedtext = text % tuple(
-                cgi.escape(unicode(replacement))
-                for replacement in replacements)
+            escaped = []
+            for replacement in replacements:
+                if isinstance(replacement, structured):
+                    escaped.append(unicode(replacement.escapedtext))
+                else:
+                    escaped.append(cgi.escape(unicode(replacement)))
+            self.escapedtext = text % tuple(escaped)
         elif kwreplacements:
-            self.escapedtext = text % dict(
-                (key, cgi.escape(unicode(value)))
-                for key, value in kwreplacements.iteritems())
+            escaped = {}
+            for key, value in kwreplacements.iteritems():
+                if isinstance(value, structured):
+                    escaped[key] = unicode(value.escapedtext)
+                else:
+                    escaped[key] = cgi.escape(unicode(value))
+            self.escapedtext = text % escaped
         else:
             self.escapedtext = unicode(text)
 

=== modified file 'lib/lp/soyuz/browser/publishing.py'
--- lib/lp/soyuz/browser/publishing.py	2010-09-06 20:19:45 +0000
+++ lib/lp/soyuz/browser/publishing.py	2011-03-01 05:06:15 +0000
@@ -20,7 +20,11 @@
 from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
 from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
-from canonical.launchpad.webapp.publisher import LaunchpadView
+from canonical.launchpad.webapp.menu import structured
+from canonical.launchpad.webapp.publisher import (
+    canonical_url,
+    LaunchpadView,
+    )
 from lp.services.propertycache import cachedproperty
 from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.interfaces.binarypackagebuild import BuildSetStatus
@@ -343,6 +347,28 @@
 
         return check_permission('launchpad.View', archive)
 
+    @property
+    def recipe_build_details(self): 
+        """Return a linkified string containing details about a
+        SourcePackageRecipeBuild.
+        """
+        if self.is_source is False:
+            return None
+        sprb = self.context.sourcepackagerelease.source_package_recipe_build
+        if sprb is not None:
+            if sprb.recipe is None:
+                recipe = 'deleted recipe'
+            else:
+                recipe = structured(
+                    'recipe <a href="%s">%s</a>',
+                    canonical_url(sprb.recipe), sprb.recipe.name)
+            return structured(
+                '<a href="%s">Built</a> by %s for <a href="%s">%s</a>',
+                    canonical_url(sprb), recipe,
+                    canonical_url(sprb.requester),
+                    sprb.requester.displayname).escapedtext
+        return None
+
 
 class SourcePublishingRecordSelectableView(SourcePublishingRecordView):
     """View class for a selectable `ISourcePackagePublishingHistory`."""

=== modified file 'lib/lp/soyuz/browser/tests/test_publishing.py'
--- lib/lp/soyuz/browser/tests/test_publishing.py	2011-02-28 04:54:56 +0000
+++ lib/lp/soyuz/browser/tests/test_publishing.py	2011-03-01 05:06:15 +0000
@@ -57,8 +57,7 @@
         spph.sourcepackagerelease.source_package_recipe_build = sprb
         recipe_link_matches = soupmatchers.HTMLContains(
             soupmatchers.Tag(
-                'link to build', 'a',
-                attrs={'href': canonical_url(sprb, force_local_path=True)},
+                'link to build', 'a', attrs={'href': canonical_url(sprb)},
                 text='Built'),
             soupmatchers.Tag(
                 'recipe name', 'a', attrs={'href': canonical_url(recipe)},
@@ -66,7 +65,7 @@
             soupmatchers.Tag(
                 'requester', 'a',
                 attrs={
-                    'href': canonical_url(requester, force_local_path=True)},
+                    'href': canonical_url(requester)},
                 text=requester.displayname))
         browser = self.getViewBrowser(spph, '+listing-archive-extra')
         self.assertThat(browser.contents, recipe_link_matches)
@@ -94,12 +93,12 @@
         recipe_link_matches = soupmatchers.HTMLContains(
             soupmatchers.Tag(
                 'link to build', 'a',
-                attrs={'href': canonical_url(sprb, force_local_path=True)},
+                attrs={'href': canonical_url(sprb)},
                 text='Built'),
             soupmatchers.Tag(
                 'requester', 'a',
                 attrs={
-                    'href': canonical_url(requester, force_local_path=True)},
+                    'href': canonical_url(requester)},
                 text=requester.displayname))
         browser = self.getViewBrowser(spph, '+listing-archive-extra')
         self.assertThat(browser.contents, recipe_link_matches)

=== modified file 'lib/lp/soyuz/templates/packagepublishing-details.pt'
--- lib/lp/soyuz/templates/packagepublishing-details.pt	2011-02-28 04:28:42 +0000
+++ lib/lp/soyuz/templates/packagepublishing-details.pt	2011-03-01 05:06:15 +0000
@@ -28,18 +28,7 @@
             tal:content="context/datesuperseded/fmt:displaydate"/>
       by <span tal:replace="context/supersededby/title" />
     </li>
-    <tal:sourcepackagerecipe condition="view/is_source">
-      <li tal:define="sprb context/sourcepackagerelease/source_package_recipe_build" tal:condition="sprb">
-        <a tal:attributes="href sprb/fmt:url">Built</a> by
-        <tal:existingrecipe condition="sprb/recipe">
-         recipe <a tal:attributes="href sprb/recipe/fmt:url" tal:content="sprb/recipe/name" ></a>
-        </tal:existingrecipe>
-        <tal:deletedrecipe condition="not: sprb/recipe">
-         deleted recipe
-        </tal:deletedrecipe>
-         for <a tal:attributes="href sprb/requester/fmt:url" tal:content="sprb/requester/displayname" ></a>.
-      </li>
-    </tal:sourcepackagerecipe>
+    <li tal:content="structure view/recipe_build_details" />
     <li tal:condition="context/datepublished">
       Published
       <span tal:attributes="title context/datepublished/fmt:datetime"