Hi Curtis,
Thanks for the review. I have merged in Brad's branch and pointed
the "Configure product branch" links at +setbranch instead of
+linkbranch, and I have responded to your comments below.
-Edwin
> Hi Edwin.
>
> I have a couple suggestion and questions that I think will take a few minutes
> to resolve.
>
> Using the UI:
>
> * "bug tracker" is two words.
> * Maybe "Configure support tracking" for answers.
> * We are asking users to Configure project development branch? I guess
> length is the issue here.
Fixed.
>
> Code:
>
> I see these warnings:
>
> == Pyflakes notices ==
>
> lib/lp/registry/browser/pillar.py
> 28: 'IProductSeries' imported but unused
>
> lib/lp/registry/browser/product.py
> 9: undefined name 'ProductInvolvementView' in __all__
>
TODO: lint
>
> > === modified file 'lib/lp/bugs/templates/bugtarget-bugs.pt'
> > --- lib/lp/bugs/templates/bugtarget-bugs.pt 2010-03-04 15:09:10 +0000
> > +++ lib/lp/bugs/templates/bugtarget-bugs.pt 2010-03-19 16:20:16 +0000
> > @@ -169,9 +169,14 @@
> > bug tracking.
> >
> > > - id="no-malone-edit">
> > - Enable
> > - bug tracking.
> > + id="no-malone-edit"
> > + tal:define ="configure_bugtracker
> context/menu:overview/configure_bugtracker | nothing"
> > + >
> > + > + tal:replace="structure configure_bugtracker/fmt:link"/>
> > + > + tal:attributes="href string:${context/fmt:url/+edit}">
> > + Enable bug tracking.
>
> Why isn't this using the context's overview menu edit link that has
> permission checking for rendering.
>
> context/menu:overview/edit:fmt:url
>
> The language looks wrong. We are interested in Configuring, not enabling.
> I think this is for the bugs root page which may be fine in this case.
I was afraid of breaking a bunch more tests if I changed the name of
the link from "Enable bug tracking", not to mention that there may be
push back on the "Configure bug tracker" link later on.
> > === modified file 'lib/lp/code/stories/branches/xx-product-branches.txt'
> > --- lib/lp/code/stories/branches/xx-product-branches.txt 2010-01-08
> 14:22:42 +0000
> > +++ lib/lp/code/stories/branches/xx-product-branches.txt 2010-03-19
> 16:20:16 +0000
> > @@ -170,7 +170,7 @@
> > the 'Import your project' button is not shown.
> >
> > >>> admin_browser.open('http://launchpad.dev/gnome-terminal')
> > - >>> admin_browser.getLink('Change details').click()
> > + >>> admin_browser.getLink('Configure Launchpad Branches').click()
> > >>> admin_browser.getControl(
> > ... 'Code for this project is published in Bazaar branches '
> > ... 'on Launchpad').click()
>
> I do not like the use of "Launchpad" in these links. They imply the project
> is setting up hosting when we are really interested in gathering project info.
>
> I have dealt with several angry, confused users who see Launchpad masquerading
> or is forking project. I think taking "Launchpad" out of the link will make
> it clear we want the know where the upstream branch is.
Removed "Launchpad" from the link text.
> > === modified file 'lib/lp/registry/browser/pillar.py'
> > --- lib/lp/registry/browser/pillar.py 2010-02-17 11:19:42 +0000
> > +++ lib/lp/registry/browser/pillar.py 2010-03-19 16:20:16 +0000
>
> > @@ -128,6 +130,78 @@
> > link for link in menuapi.navigation.values() if link.enabled],
> > key=attrgetter('sort_key'))
> >
> > + @cachedproperty
> > + def visible_disabled_links(self):
> > + """Important disabled links.
> > +
> > + These are displayed to notify the user to provide configuration
> > + info to enable the links.
> > +
> > + Override the visible_disabled_link_names attribute to change
> > + the results.
> > + """
> > + involved_menu = MenuAPI(self).navigation
> > + important_links = [
> > + involved_menu[name]
> > + for name in self.visible_disabled_link_names]
> > + return sorted([
> > + link for link in important_links if not link.enabled],
> > + key=attrgetter('sort_key'))
> +
>
> Are the "[]" required? sorted() accepts a generator which is what the first
> argument provides.
Yes, it's necessary. It's a python syntax error if the generator
expression is not the sole argument.
> > +# This class can't be moved into the browser/product.py file, since
> > +# the pillar-views.txt test will fail due to the MenuAPI adapter
> > +# for PillarView.enabled_links not working.
> > +class ProductInvolvementView(PillarView):
> > + """Encourage configuration of involvement links for projects."""
> > +
> > + has_involvement = True
> > + visible_disabled_link_names = ['submit_code']
> > +
> > + def __init__(self, context, request):
> > + super(ProductInvolvementView, self).__init__(context, request)
> > + self.official_codehosting = (
> > + self.context.development_focus.branch is not None)
> > +
> > + @property
> > + def configuration_links(self):
> > + """The enabled involvement links."""
> > + overview_menu = MenuAPI(self.context).overview
> > + series_menu = MenuAPI(self.context.development_focus).overview
> > + configuration_names = [
> > + 'configure_answers',
> > + 'configure_bugtracker',
> > + 'configure_translations',
> > + ]
> > + configuration_links = [
> > + overview_menu[name] for name in configuration_names]
> > + link_branch = series_menu['link_branch']
> > + link_branch.text = 'Configure project branch'
> > + configuration_links.append(link_branch)
> > + return sorted([
> > + link for link in configuration_links if link.enabled],
> > + key=attrgetter('sort_key'))
>
> Are the brackets required in this sorted()?
Yes.
> > === modified file 'lib/lp/registry/browser/product.py'
> > --- lib/lp/registry/browser/product.py 2010-03-09 00:46:37 +0000
> > +++ lib/lp/registry/browser/product.py 2010-03-19 16:20:16 +0000
>
> > @@ -331,6 +337,30 @@
> > return Link('+edit', text, icon='edit')
> >
> > @enabled_with_permission('launchpad.Edit')
> > + def configure_bugtracker(self):
> > + text = 'Configure project bugtracker'
> > + summary = 'Allow users to report bugs on this project'
> > + return Link('+configure-bugtracker', text, summary, icon='edit')
>
> Our intent it to know where bugs are tracked, and possibly use launchpad.
> The problem we have at this moment is that it is not possible to set the
> upstream contact without also enabling launchpad bug tracking...making it
> impossible for Ubuntu to who to contact when the project does not use
> launchpad bugs.
Changed to "Specify where bugs are tracked for this project".
> > + @enabled_with_permission('launchpad.Edit')
> > + def configure_answers(self):
> > + text = 'Configure project answers'
> > + summary = 'Allow users to ask questions on this project'
> > + return Link('+configure-answers', text, summary, icon='edit')
>
> Maybe we want to use the generic term "support tracker". Answers was called
> Support and questions "tickets" in Launchpad beta.
Done.
> > + @enabled_with_permission('launchpad.Edit')
> > + def configure_blueprints(self):
> > + text = 'Configure project blueprints'
> > + summary = 'Allow users to submit blueprints on this project'
> > + return Link('+configure-blueprints', text, summary, icon='edit')
>
> Specifications and meetings tracking is what this enables.
Changed to "Enable tracking of specifications and meetings".
> > === modified file 'lib/lp/registry/stories/product/xx-product-launchpad-
> usage.txt'
> > --- lib/lp/registry/stories/product/xx-product-launchpad-usage.txt
> 2010-01-20 23:10:13 +0000
> > +++ lib/lp/registry/stories/product/xx-product-launchpad-usage.txt
> 2010-03-19 16:20:16 +0000
> > @@ -28,20 +28,14 @@
> > >>> registrant_browser = setupBrowser(
> > ... auth='Basic :test')
> > >>> registrant_browser.open('http://launchpad.dev/firefox')
> > - >>> registrant_browser.getLink('Change details').click()
> > + >>> registrant_browser.getLink('Configure Launchpad Bugs').click()
>
> I think users will provide the information is we use common terms:
>
> Configure bug tracking
Changed.
Incremental diff:
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2010-03-29 15:46:10 +0000
+++ lib/lp/registry/browser/product.py 2010-03-29 21:29:50 +0000
@@ -338,26 +338,26 @@
@enabled_with_permission('launchpad.Edit')
def configure_bugtracker(self):
- text = 'Configure project bugtracker'
- summary = 'Allow users to report bugs on this project'
+ text = 'Configure bug tracker'
+ summary = 'Specify where bugs are tracked for this project'
return Link('+configure-bugtracker', text, summary, icon='edit')
@enabled_with_permission('launchpad.Edit')
def configure_translations(self):
- text = 'Configure project translations'
+ text = 'Configure translations'
summary = 'Allow users to submit translations for this project'
return Link('+configure-translations', text, summary, icon='edit')
@enabled_with_permission('launchpad.Edit')
def configure_answers(self):
- text = 'Configure project answers'
+ text = 'Configure support tracker'
summary = 'Allow users to ask questions on this project'
return Link('+configure-answers', text, summary, icon='edit')
@enabled_with_permission('launchpad.Edit')
def configure_blueprints(self):
- text = 'Configure project blueprints'
- summary = 'Allow users to submit blueprints on this project'
+ text = 'Configure blueprints'
+ summary = 'Enable tracking of specifications and meetings'
return Link('+configure-blueprints', text, summary, icon='edit')
@enabled_with_permission('launchpad.Edit')
=== modified file 'lib/lp/bugs/stories/bugs/xx-front-page-info.txt'
--- lib/lp/bugs/stories/bugs/xx-front-page-info.txt 2010-03-12 14:26:46 +0000
+++ lib/lp/bugs/stories/bugs/xx-front-page-info.txt 2010-04-09 16:19:25 +0000
@@ -31,7 +31,7 @@
>>> enable_tracker = find_tag_by_id(
... admin_browser.contents, 'no-malone-edit')
>>> print extract_text(enable_tracker)
- Enable bug tracking.
+ Configure bug tracker
The bugs home page for a project using Launchpad for bug tracking
shows controls for setting bug supervisor and states that no
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py 2010-03-31 20:25:33 +0000
+++ lib/lp/registry/browser/pillar.py 2010-04-09 02:25:21 +0000
@@ -25,6 +25,7 @@
from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage)
from lp.registry.interfaces.pillar import IPillar
+from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.productseries import IProductSeries
from lp.registry.interfaces.projectgroup import IProjectGroup
@@ -174,9 +175,9 @@
]
configuration_links = [
overview_menu[name] for name in configuration_names]
- link_branch = series_menu['link_branch']
- link_branch.text = 'Configure project branch'
- configuration_links.append(link_branch)
+ set_branch = series_menu['set_branch']
+ set_branch.text = 'Configure project branch'
+ configuration_links.append(set_branch)
return sorted([
link for link in configuration_links if link.enabled],
key=attrgetter('sort_key'))
@@ -197,6 +198,6 @@
def configuration_links(self):
"""The enabled involvement links."""
series_menu = MenuAPI(self.context).overview
- link_branch = series_menu['link_branch']
- link_branch.text = 'Configure series branch'
- return [link_branch]
+ set_branch = series_menu['set_branch']
+ set_branch.text = 'Configure series branch'
+ return [set_branch]
=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py 2010-04-08 19:19:13 +0000
+++ lib/lp/registry/browser/productseries.py 2010-04-09 02:25:21 +0000
@@ -176,6 +176,7 @@
links = [
'edit', 'delete', 'driver', 'link_branch', 'branch_add', 'ubuntupkg',
'create_milestone', 'create_release', 'rdf', 'subscribe',
+ 'set_branch',
]
@enabled_with_permission('launchpad.Edit')
@@ -212,6 +213,21 @@
summary = 'Change the branch for this series'
return Link('+linkbranch', text, summary, icon=icon)
+ @enabled_with_permission('launchpad.Edit')
+ def set_branch(self):
+ """Return a link to set the bazaar branch for this series."""
+ # Once +setbranch has been beta tested thoroughly, it should
+ # replace the +linkbranch page.
+ if self.context.branch is None:
+ text = 'Link to branch'
+ icon = 'add'
+ summary = 'Set the branch for this series'
+ else:
+ text = "Change branch"
+ icon = 'edit'
+ summary = 'Change the branch for this series'
+ return Link('+setbranch', text, summary, icon=icon)
+
def branch_add(self):
text = 'Register a branch'
summary = "Register a new Bazaar branch for this series' project"
@@ -735,7 +751,8 @@
)
-class ProductSeriesSetBranchView(LaunchpadFormView, ProductSeriesView,
+class ProductSeriesSetBranchView(ReturnToReferrerMixin, LaunchpadFormView,
+ ProductSeriesView,
BranchNameValidationMixin):
"""The view to set a branch for the ProductSeries."""
@@ -920,7 +937,6 @@
@action(_('Update'), name='update')
def update_action(self, action, data):
- self.next_url = canonical_url(self.context)
branch_type = data.get('branch_type')
if branch_type == LINK_LP_BZR:
branch_location = data.get('branch_location')
@@ -1012,11 +1028,6 @@
self._setBranchExists(e.existing_branch, 'branch_name')
return branch
- @property
- def cancel_url(self):
- """See `LaunchpadFormView`."""
- return canonical_url(self.context)
-
class ProductSeriesLinkBranchView(ReturnToReferrerMixin,
ProductSeriesView,
=== modified file 'lib/lp/registry/browser/tests/pillar-views.txt'
--- lib/lp/registry/browser/tests/pillar-views.txt 2010-03-31 20:25:33 +0000
+++ lib/lp/registry/browser/tests/pillar-views.txt 2010-04-09 02:25:21 +0000
@@ -85,8 +85,8 @@
... print link.name
configure_answers
configure_bugtracker
- link_branch
configure_translations
+ set_branch
Project are supported too, but they only display the applications used by
their products.
=== modified file 'lib/lp/registry/doc/product.txt'
--- lib/lp/registry/doc/product.txt 2009-12-13 11:55:40 +0000
+++ lib/lp/registry/doc/product.txt 2010-04-09 16:19:25 +0000
@@ -455,12 +455,15 @@
== Products With Branches ==
-Products can now specify whether they officially support Launchpad as a
-location for their branches.
+Products are considered to officially support Launchpad as a location
+for their branches after a branch is set for the development focus
+series.
+ >>> print firefox.development_focus.branch
+ None
>>> print firefox.official_codehosting
False
- >>> firefox.official_codehosting = True
+ >>> firefox.development_focus.branch = factory.makeBranch(product=firefox)
>>> print firefox.official_codehosting
True
=== modified file 'lib/lp/registry/stories/product/xx-product-development-focus.txt'
--- lib/lp/registry/stories/product/xx-product-development-focus.txt 2009-11-18 04:51:08 +0000
+++ lib/lp/registry/stories/product/xx-product-development-focus.txt 2010-04-09 16:19:25 +0000
@@ -32,6 +32,12 @@
... print content
... title = a.get('title', '')
... print "%s (%s)" % (title, a['href'])
+ >>> def print_involvement_portlet(browser):
+ ... involvement = find_tag_by_id(browser.contents, 'involvement')
+ ... for a in involvement.findAll('a'):
+ ... for content in a.contents:
+ ... print content
+ ... print a['href']
Projects without development focus branches
@@ -39,48 +45,59 @@
If the project has not specified a development focus branch then the
development focus section just contains a link to the development focus
-series, and text telling the user that no branch has been specified.
+series.
>>> anon_browser.open('http://launchpad.dev/fooix')
>>> print_development_focus(anon_browser)
Development focus:
trunk series
- Development focus branch has not been specified
Links:
trunk series (/fooix/trunk)
+ >>> print_involvement_portlet(anon_browser)
Setting the development focus branch
------------------------------------
If the user has rights to change the development focus or to specify the
-development focus branch, then these links are shown next to the text.
+development focus branch, then these links are shown in the involvement
+portlet.
>>> owner_browser.open('http://launchpad.dev/fooix')
>>> print_development_focus(owner_browser)
Development focus:
trunk series Change details
- Development focus branch has not been specified Link to branch
Links:
- trunk series (/fooix/trunk)
- Change details
- (http://launchpad.dev/fooix/+edit)
- Link to branch
- Set the branch for this series
- (http://launchpad.dev/fooix/trunk/+linkbranch)
+ trunk series
+ (/fooix/trunk)
+ Change details
+ (http://launchpad.dev/fooix/+edit)
+ >>> print_involvement_portlet(owner_browser)
+ Configure support tracker
+ http://launchpad.dev/fooix/+configure-answers
+ Configure bug tracker
+ http://launchpad.dev/fooix/+configure-bugtracker
+ Configure translations
+ http://launchpad.dev/fooix/+configure-translations
+ Configure project branch
+ http://launchpad.dev/fooix/trunk/+setbranch
The owner can specify the development focus branch from the overview page.
- >>> owner_browser.getLink(url='+linkbranch').click()
- >>> owner_browser.getControl('Branch').value = '~eric/fooix/trunk'
+ >>> owner_browser.getLink(url='+setbranch').click()
+ >>> owner_browser.getControl(name='field.branch_location').value = (
+ ... '~eric/fooix/trunk')
>>> owner_browser.getControl('Update').click()
>>> print_feedback_messages(owner_browser.contents)
Series code location updated.
-The owner is taken back to the trunk series page.
+The owner is taken back to the project page.
- >>> print_tag_with_id(owner_browser.contents, 'branch-details')
- lp://dev/fooix - Eric Change branch
+ >>> print_tag_with_id(owner_browser.contents, 'dev-focus')
+ Development focus:
+ trunk series Change details
+ lp://dev/fooix Change branch
+ View the branch content
Projects with development focus branches
@@ -137,7 +154,6 @@
>>> print_development_focus(anon_browser)
Development focus:
trunk series
- Development focus branch has not been specified
Links:
trunk series (/fooix/trunk)
=== modified file 'lib/lp/registry/stories/product/xx-product-launchpad-usage.txt'
--- lib/lp/registry/stories/product/xx-product-launchpad-usage.txt 2010-04-01 13:31:28 +0000
+++ lib/lp/registry/stories/product/xx-product-launchpad-usage.txt 2010-04-09 02:25:21 +0000
@@ -48,7 +48,7 @@
We'll also set it as officially using codehosting.
>>> registrant_browser.getLink('Configure project branch').click()
- >>> registrant_browser.getControl(name='field.branch').value = (
+ >>> registrant_browser.getControl(name='field.branch_location').value = (
... '~name12/firefox/main')
>>> registrant_browser.getControl('Update').click()