launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-728673 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #728673 in Launchpad itself: "BuilderSet:+index crash (/builders)"
  https://bugs.launchpad.net/launchpad/+bug/728673

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-728673/+merge/84205

SourcePackageRecipeBuilds had a custom fmt:link, separate from the PackageBuild one (used for BinaryPackageBuilds). Most notably, SourcePackageRecipeBuildFormatterAPI lacked a permission check, causing it to crash in various ways if the build was inaccessible.

This branch drops SourcePackageRecipeBuildFormatterAPI, changes PackageBuildFormatterAPI to say "private job" instead of "private source", and adds a test to confirm that inaccessible SPRBs don't crash.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-728673/+merge/84205
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-728673 into lp:launchpad.

=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2011-11-16 00:09:49 +0000
+++ lib/lp/app/browser/tales.py	2011-12-02 04:47:28 +0000
@@ -1695,7 +1695,7 @@
     def link(self, view_name, rootsite=None):
         build = self._context
         if not check_permission('launchpad.View', build):
-            return 'private source'
+            return 'private job'
 
         url = self.url(view_name=view_name, rootsite=rootsite)
         title = cgi.escape(build.title)

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2011-11-27 01:16:55 +0000
+++ lib/lp/code/browser/configure.zcml	2011-12-02 04:47:28 +0000
@@ -1097,12 +1097,6 @@
         attribute_to_parent="archive"
         path_expression="string:+recipebuild/${id}"
         />
-    <adapter
-        for="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuild"
-        provides="zope.traversing.interfaces.IPathAdapter"
-        factory="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildFormatterAPI"
-        name="fmt"
-        />
     <browser:menus
         classes="SourcePackageRecipeBuildContextMenu"
         module="lp.code.browser.sourcepackagerecipebuild"/>

=== modified file 'lib/lp/code/browser/sourcepackagerecipebuild.py'
--- lib/lp/code/browser/sourcepackagerecipebuild.py	2011-01-14 17:01:37 +0000
+++ lib/lp/code/browser/sourcepackagerecipebuild.py	2011-12-02 04:47:28 +0000
@@ -29,7 +29,6 @@
     action,
     LaunchpadFormView,
     )
-from lp.app.browser.tales import CustomizableFormatter
 from lp.buildmaster.enums import BuildStatus
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuild,
@@ -45,17 +44,6 @@
     BuildStatus.FAILEDTOUPLOAD,)
 
 
-class SourcePackageRecipeBuildFormatterAPI(CustomizableFormatter):
-    """Adapter providing fmt support for ISourcePackageRecipeBuild objects."""
-
-    _link_summary_template = '%(title)s [%(owner)s/%(archive)s]'
-
-    def _link_summary_values(self):
-        return {'title': self._context.title,
-                'owner': self._context.archive.owner.name,
-                'archive': self._context.archive.name}
-
-
 class SourcePackageRecipeBuildNavigation(Navigation, FileNavigationMixin):
 
     usedfor = ISourcePackageRecipeBuild

=== modified file 'lib/lp/code/browser/tests/test_tales.py'
--- lib/lp/code/browser/tests/test_tales.py	2011-08-12 11:37:08 +0000
+++ lib/lp/code/browser/tests/test_tales.py	2011-12-02 04:47:28 +0000
@@ -191,7 +191,7 @@
         self.assertThat(
             adapter.link(None),
             Equals(
-                '<a href="%s">%s recipe build [eric/ppa]</a>'
+                '<a href="%s">%s recipe build</a> [eric/ppa]'
                 % (canonical_url(build, path_only_if_possible=True),
                    build.recipe.base_branch.unique_name)))
 
@@ -206,5 +206,12 @@
         self.assertThat(
             adapter.link(None),
             Equals(
-                '<a href="%s">build for deleted recipe [eric/ppa]</a>'
+                '<a href="%s">build for deleted recipe</a> [eric/ppa]'
                 % (canonical_url(build, path_only_if_possible=True), )))
+
+    def test_link_no_permission(self):
+        eric = self.factory.makePerson(name='eric')
+        ppa = self.factory.makeArchive(owner=eric, name='ppa', private=True)
+        build = self.factory.makeSourcePackageRecipeBuild(archive=ppa)
+        adapter = queryAdapter(build, IPathAdapter, 'fmt')
+        self.assertThat(adapter.link(None), Equals('private job'))

=== modified file 'lib/lp/soyuz/stories/soyuz/xx-private-builds.txt'
--- lib/lp/soyuz/stories/soyuz/xx-private-builds.txt	2011-05-03 02:39:30 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-private-builds.txt	2011-12-02 04:47:28 +0000
@@ -69,7 +69,7 @@
     >>> print extract_text(find_main_content(anon_browser.contents))
     The frog builder...
     Current status
-    Building private source
+    Building private job
     ...
 
 Launchpad Administrators are allowed to see the build:
@@ -89,7 +89,7 @@
     >>> print extract_text(find_main_content(name12_browser.contents))
     The frog builder...
     Current status
-    Building private source
+    Building private job
     ...
 
 cprov is also allowed to see his own build:
@@ -215,7 +215,7 @@
     ...
     Name  Processor  Status
     bob   386        Building i386 build of mozilla-firefox ...
-    frog  386        Building private source
+    frog  386        Building private job
     ...
 
 cprov can see his own private build:
@@ -238,7 +238,7 @@
     ...
     Name  Processor  Status
     bob   386        Building i386 build of mozilla-firefox ...
-    frog  386        Building private source
+    frog  386        Building private job
     ...