Merge lp:~wgrant/launchpad/bug-728673 into lp:launchpad

Proposed by William Grant
Status: Merged
Approved by: William Grant
Approved revision: no longer in the source branch.
Merged at revision: 14427
Proposed branch: lp:~wgrant/launchpad/bug-728673
Merge into: lp:launchpad
Diff against target: 124 lines (+14/-25)
5 files modified
lib/lp/app/browser/tales.py (+1/-1)
lib/lp/code/browser/configure.zcml (+0/-6)
lib/lp/code/browser/sourcepackagerecipebuild.py (+0/-12)
lib/lp/code/browser/tests/test_tales.py (+9/-2)
lib/lp/soyuz/stories/soyuz/xx-private-builds.txt (+4/-4)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-728673
Reviewer Review Type Date Requested Status
Ian Booth (community) code Approve
Review via email: mp+84205@code.launchpad.net

Commit message

[r=wallyworld][bug=728673] Don't crash when calling fmt:link on an inaccessible SourcePackageRecipeBuild.

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Ian Booth (wallyworld) wrote :

Any change which deletes code is good :-)
Nice cleanup also.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/browser/tales.py'
2--- lib/lp/app/browser/tales.py 2011-11-16 00:09:49 +0000
3+++ lib/lp/app/browser/tales.py 2011-12-02 04:47:28 +0000
4@@ -1695,7 +1695,7 @@
5 def link(self, view_name, rootsite=None):
6 build = self._context
7 if not check_permission('launchpad.View', build):
8- return 'private source'
9+ return 'private job'
10
11 url = self.url(view_name=view_name, rootsite=rootsite)
12 title = cgi.escape(build.title)
13
14=== modified file 'lib/lp/code/browser/configure.zcml'
15--- lib/lp/code/browser/configure.zcml 2011-11-27 01:16:55 +0000
16+++ lib/lp/code/browser/configure.zcml 2011-12-02 04:47:28 +0000
17@@ -1097,12 +1097,6 @@
18 attribute_to_parent="archive"
19 path_expression="string:+recipebuild/${id}"
20 />
21- <adapter
22- for="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuild"
23- provides="zope.traversing.interfaces.IPathAdapter"
24- factory="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildFormatterAPI"
25- name="fmt"
26- />
27 <browser:menus
28 classes="SourcePackageRecipeBuildContextMenu"
29 module="lp.code.browser.sourcepackagerecipebuild"/>
30
31=== modified file 'lib/lp/code/browser/sourcepackagerecipebuild.py'
32--- lib/lp/code/browser/sourcepackagerecipebuild.py 2011-01-14 17:01:37 +0000
33+++ lib/lp/code/browser/sourcepackagerecipebuild.py 2011-12-02 04:47:28 +0000
34@@ -29,7 +29,6 @@
35 action,
36 LaunchpadFormView,
37 )
38-from lp.app.browser.tales import CustomizableFormatter
39 from lp.buildmaster.enums import BuildStatus
40 from lp.code.interfaces.sourcepackagerecipebuild import (
41 ISourcePackageRecipeBuild,
42@@ -45,17 +44,6 @@
43 BuildStatus.FAILEDTOUPLOAD,)
44
45
46-class SourcePackageRecipeBuildFormatterAPI(CustomizableFormatter):
47- """Adapter providing fmt support for ISourcePackageRecipeBuild objects."""
48-
49- _link_summary_template = '%(title)s [%(owner)s/%(archive)s]'
50-
51- def _link_summary_values(self):
52- return {'title': self._context.title,
53- 'owner': self._context.archive.owner.name,
54- 'archive': self._context.archive.name}
55-
56-
57 class SourcePackageRecipeBuildNavigation(Navigation, FileNavigationMixin):
58
59 usedfor = ISourcePackageRecipeBuild
60
61=== modified file 'lib/lp/code/browser/tests/test_tales.py'
62--- lib/lp/code/browser/tests/test_tales.py 2011-08-12 11:37:08 +0000
63+++ lib/lp/code/browser/tests/test_tales.py 2011-12-02 04:47:28 +0000
64@@ -191,7 +191,7 @@
65 self.assertThat(
66 adapter.link(None),
67 Equals(
68- '<a href="%s">%s recipe build [eric/ppa]</a>'
69+ '<a href="%s">%s recipe build</a> [eric/ppa]'
70 % (canonical_url(build, path_only_if_possible=True),
71 build.recipe.base_branch.unique_name)))
72
73@@ -206,5 +206,12 @@
74 self.assertThat(
75 adapter.link(None),
76 Equals(
77- '<a href="%s">build for deleted recipe [eric/ppa]</a>'
78+ '<a href="%s">build for deleted recipe</a> [eric/ppa]'
79 % (canonical_url(build, path_only_if_possible=True), )))
80+
81+ def test_link_no_permission(self):
82+ eric = self.factory.makePerson(name='eric')
83+ ppa = self.factory.makeArchive(owner=eric, name='ppa', private=True)
84+ build = self.factory.makeSourcePackageRecipeBuild(archive=ppa)
85+ adapter = queryAdapter(build, IPathAdapter, 'fmt')
86+ self.assertThat(adapter.link(None), Equals('private job'))
87
88=== modified file 'lib/lp/soyuz/stories/soyuz/xx-private-builds.txt'
89--- lib/lp/soyuz/stories/soyuz/xx-private-builds.txt 2011-05-03 02:39:30 +0000
90+++ lib/lp/soyuz/stories/soyuz/xx-private-builds.txt 2011-12-02 04:47:28 +0000
91@@ -69,7 +69,7 @@
92 >>> print extract_text(find_main_content(anon_browser.contents))
93 The frog builder...
94 Current status
95- Building private source
96+ Building private job
97 ...
98
99 Launchpad Administrators are allowed to see the build:
100@@ -89,7 +89,7 @@
101 >>> print extract_text(find_main_content(name12_browser.contents))
102 The frog builder...
103 Current status
104- Building private source
105+ Building private job
106 ...
107
108 cprov is also allowed to see his own build:
109@@ -215,7 +215,7 @@
110 ...
111 Name Processor Status
112 bob 386 Building i386 build of mozilla-firefox ...
113- frog 386 Building private source
114+ frog 386 Building private job
115 ...
116
117 cprov can see his own private build:
118@@ -238,7 +238,7 @@
119 ...
120 Name Processor Status
121 bob 386 Building i386 build of mozilla-firefox ...
122- frog 386 Building private source
123+ frog 386 Building private job
124 ...
125
126