Merge lp://staging/~bac/launchpad/bug-490521 into lp://staging/launchpad
- bug-490521
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | not available |
Merged at revision: | not available |
Proposed branch: | lp://staging/~bac/launchpad/bug-490521 |
Merge into: | lp://staging/launchpad |
Diff against target: |
295 lines (+171/-36) 6 files modified
lib/lp/app/templates/base-layout-macros.pt (+7/-0) lib/lp/registry/browser/sourcepackage.py (+16/-1) lib/lp/registry/browser/tests/sourcepackage-views.txt (+81/-2) lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt (+7/-3) lib/lp/registry/templates/sourcepackage-portlet-associations.pt (+54/-27) lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt (+6/-3) |
To merge this branch: | bzr merge lp://staging/~bac/launchpad/bug-490521 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code and ui | Approve | |
Review via email: mp+19133@code.staging.launchpad.net |
Commit message
Change source package associations portlet to give an indication of whether various upstream attributes (bug tracker, bug supervisor, series branch, etc) have been configured.
Description of the change
Brad Crittenden (bac) wrote : | # |
Curtis Hovey (sinzui) wrote : | # |
...
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
...
> @@ -349,3 +348,22 @@
> 'The project %s was linked to this source package.' %
> upstream.
> self.next_url = self.request.
> +
> + @property
> + def has_bugtracker(
> + """Does the product have a bugtracker set?"""
> + def getTrackerName(
> + if bugtracker is None:
> + return False
> + else:
> + return True
> + product = self.context.
> + if product.
> + return True
> + bugtracker = product.bugtracker
> + if bugtracker is None:
> + if product.project is not None:
> + bugtracker = product.
> + if bugtracker is None:
> + return False
> + return True
I see that I underestimated the complexity of this connection. I think this
needs better test coverage. I think a test of a new factory-made product
that gets a project without a bug tracker will work. Change the
official_malone and bug tracker a few times to show that has_bugtracker
returns the expected state.
...
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -5,37 +5,51 @@
> omit-tag="">
>
> <div id="portlet-
> - <h2>Upstream associations</h2>
> + <h2>Upstream connections</h2>
I am considering a similar portlet on the project page:
Contributor connections
Is the title on this portlet clearer if it is:
Upstream contributor connections
...
> + <dl>
> + <dt>
> + <a tal:replace=
> + </dt>
We lost the link to change the linked series. This is needed by about 50
packages each release when the package is built from a new stable series.
We have a choice here: add an edit icon here:
<a tal:replace=
But I think the action will be more clear if we move it below because the user
may want to change the /project/ and series. See the end of this review.
> + <dd title="Series branch"
> + tal:define="bool series/branch">
> + Branch:
> + <metal:yes-no
> + use-macro=
> + </dd>
> + </dl>
As I said on IRC, we need to show translations too...
...and you just showed me your fix in IRC. Fab.
> </div>
>
> <ul class="horizontal">
I think we should move the link to edit the Packaging link to this list
where we can use a clear label: Change upstream project and series. As
we agreed on the phone, if the link should be different if the upstream
had never been s...
Brad Crittenden (bac) wrote : | # |
Curtis,
I have made all of the requested changes except changing the portlet title. It doesn't seem clearer to me but we can discuss it more before landing.
The incremental diff is at: http://
Thanks for the review and the great suggestions.
Curtis Hovey (sinzui) wrote : | # |
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
...
> +So let's set the product series so we can do more interesting testing.
> +
> + >>> login_person(
> + >>> form = {
> + ... 'field.
> + ... 'field.
> + ... }
> + >>> view = create_
> + ... package, name='+
> + ... principal=
> + >>> view.errors
> + []
> + >>> print package.
> + stinkyseries
This is duplicating a test from packaging-views and will break if we change
that form. Just create a packaging link so that we can focus on the property.
package.
> +If a product is not part of a project group and its bug tracker is not
> +set then the view property is false.
> +
> + >>> view = create_
> +
> + >>> print product.
> + False
> + >>> print product.bugtracker
> + None
> + >>> print view.has_bugtracker
> + False
> +
> +Having official_malone set results in has_bugtracker being true.
> +
> + >>> product.
> + >>> print view.has_bugtracker
> + True
> +
> +Having a bug_tracker set also results in has_bugtracker being true (a
> +bit of a tautology you'd think).
> +
> + >>> product.
> + >>> from zope.component import getUtility
> + >>> from canonical.
> + >>> product.bugtracker = getUtility(
> + >>> print view.has_bugtracker
> + True
This is easier. Graham got tired of looking up trackers in his tests.
product.
...
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -48,16 +48,30 @@
> <metal:yes-no
> use-macro=
> </dd>
> + <dd title="Series translations auto import"
> + tal:condition=
> + tal:define="bool not:series/
> + Translations:
> + <metal:yes-no
> + use-macro=
> + </dd>
> </dl>
>
> </div>
>
> <ul class="horizontal">
> <li>
> - <a
> - tal:attributes=
> + <a tal:attributes=
> + class string:sprite info"
> >Show upstr...
Brad Crittenden (bac) wrote : | # |
And another diff at http://
Off to land now. Thanks for feedback.
Preview Diff
1 | === modified file 'lib/lp/app/templates/base-layout-macros.pt' |
2 | --- lib/lp/app/templates/base-layout-macros.pt 2010-02-11 20:44:48 +0000 |
3 | +++ lib/lp/app/templates/base-layout-macros.pt 2010-02-16 17:50:30 +0000 |
4 | @@ -410,4 +410,11 @@ |
5 | condition="python: count != 1" |
6 | replace="l_plural" /> |
7 | </metal:plural-msg> |
8 | + |
9 | +<metal:yes-no define-macro="yes-no"> |
10 | + <span tal:condition="bool" |
11 | + class="sprite yes"> <span class="invisible-link">yes</span></span> |
12 | + <span tal:condition="not: bool" |
13 | + class="sprite no"> <span class="invisible-link">no</span></span> |
14 | +</metal:yes-no> |
15 | </macros> |
16 | |
17 | === modified file 'lib/lp/registry/browser/sourcepackage.py' |
18 | --- lib/lp/registry/browser/sourcepackage.py 2010-01-29 15:03:49 +0000 |
19 | +++ lib/lp/registry/browser/sourcepackage.py 2010-02-16 17:50:30 +0000 |
20 | @@ -28,7 +28,6 @@ |
21 | |
22 | from lazr.restful.interface import copy_field |
23 | |
24 | -from canonical.cachedproperty import cachedproperty |
25 | from canonical.widgets import LaunchpadRadioWidget |
26 | |
27 | from canonical.launchpad import helpers |
28 | @@ -349,3 +348,19 @@ |
29 | 'The project %s was linked to this source package.' % |
30 | upstream.displayname) |
31 | self.next_url = self.request.getURL() |
32 | + |
33 | + @property |
34 | + def has_bugtracker(self): |
35 | + """Does the product have a bugtracker set?""" |
36 | + if self.context.productseries is None: |
37 | + return False |
38 | + product = self.context.productseries.product |
39 | + if product.official_malone: |
40 | + return True |
41 | + bugtracker = product.bugtracker |
42 | + if bugtracker is None: |
43 | + if product.project is not None: |
44 | + bugtracker = product.project.bugtracker |
45 | + if bugtracker is None: |
46 | + return False |
47 | + return True |
48 | |
49 | === modified file 'lib/lp/registry/browser/tests/sourcepackage-views.txt' |
50 | --- lib/lp/registry/browser/tests/sourcepackage-views.txt 2010-01-29 15:03:49 +0000 |
51 | +++ lib/lp/registry/browser/tests/sourcepackage-views.txt 2010-02-16 17:50:30 +0000 |
52 | @@ -119,8 +119,11 @@ |
53 | ... extract_text, find_tag_by_id) |
54 | >>> content = extract_text(find_tag_by_id(view.render(), 'upstreams')) |
55 | >>> print content |
56 | - Project:...Bonkers... |
57 | - Series:...crazy... |
58 | + Bonkers project |
59 | + Bug supervisor: no |
60 | + Bug tracker: no |
61 | + Bonkers crazy series |
62 | + Branch: no |
63 | |
64 | A new source project that is not linked to an upstream will result in |
65 | the portlet showing the suggested project. |
66 | @@ -160,3 +163,79 @@ |
67 | Registered upstream project: |
68 | Lernid... |
69 | Lernid Dev... |
70 | + |
71 | + |
72 | +The view includes a property for determining if the project has a bug |
73 | +tracker, though the rules are somewhat complicated. |
74 | + |
75 | +If the view's package has no productseries set then has_bugtracker is False. |
76 | + |
77 | + |
78 | + >>> product = factory.makeProduct(name='stinky', displayname='Stinky') |
79 | + >>> productseries = factory.makeProductSeries( |
80 | + ... name='stinkyseries', product=product) |
81 | + >>> distroseries = factory.makeDistroRelease(name='wonky', |
82 | + ... distribution=distribution) |
83 | + >>> sourcepackagename = factory.makeSourcePackageName(name='stinkypackage') |
84 | + >>> package = factory.makeSourcePackage( |
85 | + ... sourcepackagename=sourcepackagename, distroseries=distroseries) |
86 | + |
87 | + >>> view = create_initialized_view(package, name='+portlet-associations') |
88 | + |
89 | + >>> print package.productseries |
90 | + None |
91 | + >>> print view.has_bugtracker |
92 | + False |
93 | + |
94 | +So let's set the product series so we can do more interesting testing. |
95 | + |
96 | + >>> package.setPackaging(productseries, product.owner) |
97 | + >>> print package.productseries.name |
98 | + stinkyseries |
99 | + |
100 | +If a product is not part of a project group and its bug tracker is not |
101 | +set then the view property is false. |
102 | + |
103 | + >>> view = create_initialized_view(package, name='+portlet-associations') |
104 | + |
105 | + >>> print product.official_malone |
106 | + False |
107 | + >>> print product.bugtracker |
108 | + None |
109 | + >>> print view.has_bugtracker |
110 | + False |
111 | + |
112 | +Having official_malone set results in has_bugtracker being true. |
113 | + |
114 | + >>> login_person(product.owner) |
115 | + >>> product.official_malone = True |
116 | + >>> print view.has_bugtracker |
117 | + True |
118 | + |
119 | +Having a bug_tracker set also results in has_bugtracker being true (a |
120 | +bit of a tautology you'd think). |
121 | + |
122 | + >>> product.official_malone = False |
123 | + >>> bugtracker = factory.makeBugTracker() |
124 | + >>> product.bugtracker = bugtracker |
125 | + >>> print view.has_bugtracker |
126 | + True |
127 | + |
128 | +If the product has no bug tracker and is in a project group with no |
129 | +bug tracker then the property is false. |
130 | + |
131 | + >>> product.bugtracker = None |
132 | + >>> project = factory.makeProject() |
133 | + >>> print project.bugtracker |
134 | + None |
135 | + >>> product.project = project |
136 | + >>> print view.has_bugtracker |
137 | + False |
138 | + |
139 | +If the product's project does have a bug tracker then the product |
140 | +inherits it. |
141 | + |
142 | + >>> login_person(project.owner) |
143 | + >>> project.bugtracker = bugtracker |
144 | + >>> print view.has_bugtracker |
145 | + True |
146 | |
147 | === modified file 'lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt' |
148 | --- lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt 2010-01-25 23:23:27 +0000 |
149 | +++ lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt 2010-02-16 17:50:30 +0000 |
150 | @@ -28,11 +28,15 @@ |
151 | >>> user_browser.getControl("Change").click() |
152 | >>> print extract_text(find_tag_by_id( |
153 | ... user_browser.contents, 'upstreams')) |
154 | + Mozilla Thunderbird ... project |
155 | Project Group: the Mozilla Project |
156 | - Project: Mozilla Thunderbird ... |
157 | - Series: trunk ... |
158 | + Bug supervisor: no |
159 | + Bug tracker: no |
160 | + Mozilla Thunderbird trunk series |
161 | + Branch: no |
162 | + Translations: no |
163 | |
164 | -He see the "Show upstream links" link and takes a look at the project's |
165 | +He sees the "Show upstream links" link and takes a look at the project's |
166 | packaging in distributions. |
167 | |
168 | >>> user_browser.getLink('Show upstream links').click() |
169 | |
170 | === modified file 'lib/lp/registry/templates/sourcepackage-portlet-associations.pt' |
171 | --- lib/lp/registry/templates/sourcepackage-portlet-associations.pt 2010-02-01 13:13:23 +0000 |
172 | +++ lib/lp/registry/templates/sourcepackage-portlet-associations.pt 2010-02-16 17:50:30 +0000 |
173 | @@ -5,45 +5,72 @@ |
174 | omit-tag=""> |
175 | |
176 | <div id="portlet-associations"> |
177 | - <h2>Upstream associations</h2> |
178 | + <h2>Upstream connections</h2> |
179 | |
180 | <div class="portletBody portletContent" |
181 | tal:define="series context/productseries"> |
182 | <tal:has_series condition="series"> |
183 | <div id="upstreams" class="two-column-list"> |
184 | - <dl |
185 | - tal:define="project series/product/project" |
186 | - tal:condition="project"> |
187 | - <dt>Project Group:</dt> |
188 | - <dd> |
189 | - <a tal:replace="structure project/fmt:link" /> |
190 | - </dd> |
191 | - </dl> |
192 | - |
193 | - <dl> |
194 | - <dt>Project:</dt> |
195 | - <dd> |
196 | - <a tal:replace="structure series/product/fmt:link" /> |
197 | - </dd> |
198 | - </dl> |
199 | - |
200 | - <dl> |
201 | - <dt>Series:</dt> |
202 | - <dd> |
203 | - <a |
204 | - tal:content="series/name" |
205 | - tal:attributes="href series/fmt:url">Series</a> |
206 | - <a tal:replace="structure context/menu:overview/edit_packaging/fmt:icon" /> |
207 | - </dd> |
208 | - </dl> |
209 | + <dl> |
210 | + <dt> |
211 | + <a tal:replace="structure series/product/fmt:link" /> project |
212 | + </dt> |
213 | + |
214 | + <tal:has_pg define="project series/product/project" |
215 | + condition="project"> |
216 | + <dd title="Project group"> |
217 | + Project Group: |
218 | + <a tal:replace="structure project/fmt:link" /> |
219 | + </dd> |
220 | + </tal:has_pg> |
221 | + |
222 | + <dd title="Bug supervisor" |
223 | + tal:define="bool series/product/bug_supervisor"> |
224 | + Bug supervisor: |
225 | + <metal:yes-no |
226 | + use-macro="context/@@+base-layout-macros/yes-no" /> |
227 | + </dd> |
228 | + |
229 | + <dd title="Bug tracker" |
230 | + tal:define="bool view/has_bugtracker"> |
231 | + Bug tracker: |
232 | + <metal:yes-no |
233 | + use-macro="context/@@+base-layout-macros/yes-no" /> |
234 | + </dd> |
235 | + </dl> |
236 | + <dl> |
237 | + <dt> |
238 | + <a tal:replace="structure series/fmt:link" /> |
239 | + </dt> |
240 | + <dd title="Series branch" |
241 | + tal:define="bool series/branch"> |
242 | + Branch: |
243 | + <metal:yes-no |
244 | + use-macro="context/@@+base-layout-macros/yes-no" /> |
245 | + </dd> |
246 | + <dd title="Series translations auto import" |
247 | + tal:condition="context/getTranslationTemplates" |
248 | + tal:define="bool not:series/translations_autoimport_mode/enumvalue:NO_IMPORT"> |
249 | + Translations: |
250 | + <metal:yes-no |
251 | + use-macro="context/@@+base-layout-macros/yes-no" /> |
252 | + </dd> |
253 | + </dl> |
254 | + |
255 | </div> |
256 | |
257 | <ul class="horizontal"> |
258 | <li> |
259 | - <a |
260 | + <a class="sprite info" |
261 | tal:attributes="href series/product/menu:overview/packages/fmt:url" |
262 | >Show upstream links</a> |
263 | </li> |
264 | + <li> |
265 | + <a class="sprite edit" |
266 | + tal:attributes=" |
267 | + href context/menu:overview/edit_packaging/fmt:url" |
268 | + >Change upstream project and series</a> |
269 | + </li> |
270 | </ul> |
271 | </tal:has_series> |
272 | |
273 | |
274 | === modified file 'lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt' |
275 | --- lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt 2010-01-20 14:29:59 +0000 |
276 | +++ lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt 2010-02-16 17:50:30 +0000 |
277 | @@ -69,12 +69,15 @@ |
278 | mozilla-firefox 0.9 |
279 | |
280 | It also provides the upstream relationships for this source, Project, |
281 | -Product and Branches: |
282 | +Product, Branches, and Bugs: |
283 | |
284 | >>> print extract_text(find_tag_by_id(browser.contents, 'upstreams')) |
285 | + Mozilla Firefox ... project |
286 | Project Group: the Mozilla Project |
287 | - Project: Mozilla Firefox ... |
288 | - Series: trunk Change upstream link |
289 | + Bug supervisor: no |
290 | + Bug tracker: yes |
291 | + Mozilla Firefox trunk series |
292 | + Branch: no |
293 | |
294 | The user can also download the files for the "currentrelease" (last |
295 | published version) if they are available: |
= Summary =
For a package that has been connected to an upstream project an indicator for the
different attributes is shown in the "Upstream connections" portlet.
== Proposed fix ==
Include the new 'yes-no' TALES macro written by Curtis and use it to visually
indicate whether bug supervisor, bug tracker, and the branch for the series are
populated.
== Pre-implementation notes ==
Lots of discussions with Curtis and Edwin.
== Implementation details ==
Hack in hard-coded uses of the new macro. If this UI design is accepted after
testing on edge it will be refactored into a new view so that the functionality can
be used elsewhere.
== Tests ==
bin/test -vvm lp.registry -t xx-sourcepackag e-packaging. txt \ views.txt
-t sourcepackage-
== Demo and Q/A ==
See it in action at https:/ /launchpad. dev/ubuntu/ hoary/+ source/ netapplet
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files: registry/ browser/ sourcepackage. py registry/ stories/ packaging/ xx-sourcepackag e-packaging. txt app/templates/ base-layout- macros. pt registry/ browser/ tests/sourcepac kage-views. txt registry/ templates/ sourcepackage- portlet- associations. pt
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
== Pylint notices ==
lib/lp/ registry/ browser/ sourcepackage. py interface' (No module named restful)
29: [F0401] Unable to import 'lazr.restful.