launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03192
[Merge] lp:~allenap/launchpad/dd-addseries-bug-745793 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/dd-addseries-bug-745793 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #745793 in Launchpad itself: "Simplify Distribution:+addseries"
https://bugs.launchpad.net/launchpad/+bug/745793
For more details, see:
https://code.launchpad.net/~allenap/launchpad/dd-addseries-bug-745793/+merge/56253
Simplify the Distribution:+addseries page by removing the title,
description and parent_series fields (a mock-up can be found on
https://dev.launchpad.net/LEP/DerivativeDistributions). It also adds
some pop-up help and more descriptive text.
I haven't followed the mock-up exactly. For example, where the mock-up
uses the verb "Register" I have used "Add". Links to the page are all
"Add a series", the page itself is named +addseries, and it's less
fancy too.
In the process of adding the help links I created another feature for
LaunchpadFormView. If help_links is set, these are applied to the
widgets in the form, and launchpad-form.pt then includes them in the
generated widget rows. The help_link widget attribute can also be set
with custom_widget():
custom_widget("name", TextWidget, help_link=u"/+help/...")
--
https://code.launchpad.net/~allenap/launchpad/dd-addseries-bug-745793/+merge/56253
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/dd-addseries-bug-745793 into lp:launchpad.
=== modified file 'lib/lp/app/browser/launchpadform.py'
--- lib/lp/app/browser/launchpadform.py 2010-12-13 08:23:15 +0000
+++ lib/lp/app/browser/launchpadform.py 2011-04-04 19:56:15 +0000
@@ -38,8 +38,10 @@
providedBy,
)
from zope.interface.advice import addClassAdvisor
-from zope.traversing.interfaces import ITraversable, TraversalError
-
+from zope.traversing.interfaces import (
+ ITraversable,
+ TraversalError,
+ )
from canonical.launchpad.webapp.interfaces import (
IAlwaysSubmittedWidget,
@@ -189,6 +191,13 @@
self.form_fields, self.prefix, context, self.request,
data=self.initial_values, adapters=self.adapters,
ignore_request=False)
+ for field_name, help_link in self.help_links.iteritems():
+ self.widgets[field_name].help_link = help_link
+
+ @property
+ def help_links(self):
+ """Dictionary mapping field names to help links."""
+ return {}
@property
def adapters(self):
=== modified file 'lib/lp/app/browser/tests/test_launchpadform.py'
--- lib/lp/app/browser/tests/test_launchpadform.py 2010-11-25 03:21:16 +0000
+++ lib/lp/app/browser/tests/test_launchpadform.py 2011-04-04 19:56:15 +0000
@@ -5,16 +5,23 @@
__metaclass__ = type
+from lxml import html
+from z3c.ptcompat import ViewPageTemplateFile
from zope.interface import Interface
from zope.schema import Text
+from canonical.config import config
from canonical.launchpad.webapp.servers import LaunchpadTestRequest
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.app.browser.launchpadform import (
has_structured_doc,
LaunchpadFormView,
)
-from lp.testing import TestCase, test_tales
+from lp.testing import (
+ test_tales,
+ TestCase,
+ TestCaseWithFactory,
+ )
class TestInterface(Interface):
@@ -64,3 +71,65 @@
'widget/query:has-structured-doc', widget=normal_widget))
self.assertTrue(test_tales(
'widget/query:has-structured-doc', widget=structured_widget))
+
+
+class TestHelpLinksInterface(Interface):
+ """Test interface for the view below."""
+
+ nickname = Text(title=u'name')
+
+ displayname = Text(title=u'name')
+
+
+class TestHelpLinksView(LaunchpadFormView):
+ """A trivial view that contains help links."""
+
+ schema = TestHelpLinksInterface
+
+ page_title = u"TestHelpLinksView"
+ template = ViewPageTemplateFile(
+ config.root + '/lib/lp/app/templates/generic-edit.pt')
+
+ help_links = {
+ "nickname": u"http://widget.example.com/name";,
+ "displayname": u"http://widget.example.com/displayname";,
+ }
+
+
+class TestHelpLinks(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_help_links_on_widget(self):
+ # The values in a view's help_links dictionary gets copied into the
+ # corresponding widgets' help_link attributes.
+ request = LaunchpadTestRequest()
+ view = TestHelpLinksView(None, request)
+ view.initialize()
+ nickname_widget, displayname_widget = list(view.widgets)
+ self.assertEqual(
+ u"http://widget.example.com/name";,
+ nickname_widget.help_link)
+ self.assertEqual(
+ u"http://widget.example.com/displayname";,
+ displayname_widget.help_link)
+
+ def test_help_links_render(self):
+ # The values in a view's help_links dictionary are rendered in the
+ # default generic-edit template.
+ user = self.factory.makePerson()
+ request = LaunchpadTestRequest(PATH_INFO="/")
+ request.setPrincipal(user)
+ view = TestHelpLinksView(user, request)
+ view.initialize()
+ root = html.fromstring(view.render())
+ [nickname_help_link] = root.cssselect(
+ "label[for$=nickname] ~ a[target=help]")
+ self.assertEqual(
+ u"http://widget.example.com/name";,
+ nickname_help_link.get("href"))
+ [displayname_help_link] = root.cssselect(
+ "label[for$=displayname] ~ a[target=help]")
+ self.assertEqual(
+ u"http://widget.example.com/displayname";,
+ displayname_help_link.get("href"))
=== modified file 'lib/lp/app/templates/launchpad-form.pt'
--- lib/lp/app/templates/launchpad-form.pt 2011-02-14 01:48:57 +0000
+++ lib/lp/app/templates/launchpad-form.pt 2011-04-04 19:56:15 +0000
@@ -105,7 +105,7 @@
error_class python:error and 'error' or None;
show_optional python:view.showOptionalMarker(field_name);
widget_class widget/widget_class|nothing;
- widget_help_link widget_help_link|nothing">
+ widget_help_link widget_help_link|widget/help_link|nothing">
<tal:is-visible condition="widget/visible">
<tr
tal:condition="python: view.isSingleLineLayout(field_name)"
@@ -124,7 +124,7 @@
<tal:help-link condition="widget_help_link">
<a tal:attributes="href widget_help_link"
target="help" class="sprite maybe">
- <span class="invisible-link">Tag help</span>
+ <span class="invisible-link">(?)</span>
</a>
</tal:help-link>
</div>
@@ -148,6 +148,12 @@
<span tal:condition="show_optional"
class="fieldRequired">(Optional)</span>
</tal:showlabel>
+ <tal:help-link condition="widget_help_link">
+ <a tal:attributes="href widget_help_link"
+ target="help" class="sprite maybe">
+ <span class="invisible-link">(?)</span>
+ </a>
+ </tal:help-link>
<div
tal:condition="error"
tal:content="structure error"
@@ -170,7 +176,7 @@
<tal:help-link condition="widget_help_link">
<a tal:attributes="href widget_help_link"
target="help" class="sprite maybe">
- <span class="invisible-link">Tag help</span>
+ <span class="invisible-link">(?)</span>
</a>
</tal:help-link>
<div
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2011-03-31 12:53:27 +0000
+++ lib/lp/registry/browser/configure.zcml 2011-04-04 19:56:15 +0000
@@ -1957,7 +1957,7 @@
class="lp.registry.browser.distroseries.DistroSeriesAddView"
facet="overview"
permission="launchpad.Admin"
- template="../../app/templates/generic-edit.pt">
+ template="../templates/distroseries-add.pt">
</browser:page>
<browser:page
name="+addseries"
@@ -1965,7 +1965,7 @@
class="lp.registry.browser.distroseries.DistroSeriesAddView"
facet="overview"
permission="launchpad.Append"
- template="../../app/templates/generic-edit.pt">
+ template="../templates/distroseries-add.pt">
</browser:page>
<browser:page
name="+initseries"
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-03-31 12:53:27 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-04-04 19:56:15 +0000
@@ -19,6 +19,7 @@
'DistroSeriesView',
]
+from lazr.restful.interface import copy_field
from zope.component import getUtility
from zope.event import notify
from zope.formlib import form
@@ -39,7 +40,6 @@
_,
helpers,
)
-from canonical.launchpad.interfaces.launchpad import ILaunchBag
from canonical.launchpad.webapp import (
action,
custom_widget,
@@ -504,31 +504,68 @@
self.next_url = canonical_url(self.context)
+class IDistroSeriesAddForm(Interface):
+
+ name = copy_field(
+ IDistroSeries["name"], description=_(
+ "The name of this series as used for URLs."))
+
+ version = copy_field(
+ IDistroSeries["version"], description=_(
+ "The version of the new series."))
+
+ displayname = copy_field(
+ IDistroSeries["displayname"], description=_(
+ "The name of the new series as it would "
+ "appear in a paragraph."))
+
+ summary = copy_field(IDistroSeries["summary"])
+
+
class DistroSeriesAddView(LaunchpadFormView):
"""A view to create an `IDistroSeries`."""
- schema = IDistroSeries
+ schema = IDistroSeriesAddForm
field_names = [
- 'name', 'displayname', 'title', 'summary', 'description', 'version',
- 'parent_series']
-
- label = 'Register a series'
+ 'name',
+ 'version',
+ 'displayname',
+ 'summary',
+ ]
+
+ help_links = {
+ "name": u"/+help/distribution-add-series.html#codename",
+ }
+
+ label = 'Add a series'
page_title = label
- @action(_('Create Series'), name='create')
+ def validate(self, data):
+ # XXX: Rip this out before landing.
+ deprecated_field_names = [
+ 'field.title',
+ 'field.description',
+ 'field.parent_series',
+ ]
+ deprecated_field_names_found = set(
+ self.request).intersection(deprecated_field_names)
+ if len(deprecated_field_names_found) > 0:
+ raise AssertionError(
+ "Deprecated fields: %s" % " ".join(
+ deprecated_field_names_found))
+ super(DistroSeriesAddView, self).validate(data)
+
+ @action(_('Add Series'), name='create')
def createAndAdd(self, action, data):
"""Create and add a new Distribution Series"""
- registrant = getUtility(ILaunchBag).user
-
- assert registrant is not None
distroseries = self.context.newSeries(
name=data['name'],
displayname=data['displayname'],
- title=data['title'],
+ title=data['displayname'],
summary=data['summary'],
- description=data['description'],
+ description=u"",
version=data['version'],
- parent_series=data['parent_series'],
- registrant=registrant)
+ parent_series=None,
+ registrant=self.user)
notify(ObjectCreatedEvent(distroseries))
self.next_url = canonical_url(distroseries)
return distroseries
=== modified file 'lib/lp/registry/browser/tests/distroseries-views.txt'
--- lib/lp/registry/browser/tests/distroseries-views.txt 2010-10-09 16:36:22 +0000
+++ lib/lp/registry/browser/tests/distroseries-views.txt 2011-04-04 19:56:15 +0000
@@ -86,6 +86,7 @@
Changeslist: foo@xxxxxxx
Status: DEVELOPMENT
+
When the distroseries is released, i.e. when it goes from an unstable
status (FUTURE, EXPERIMENTAL, DEVELOPMENT, FROZEN) to CURRENT, its
'datereleased' field is set.
@@ -209,9 +210,9 @@
>>> view = create_view(ubuntu, '+addseries')
>>> print view.page_title
- Register a series
+ Add a series
>>> print view.label
- Register a series
+ Add a series
>>> print view.cancel_url
http://launchpad.dev/ubuntu
@@ -219,19 +220,15 @@
None
>>> view.field_names
- ['name', 'displayname', 'title', 'summary', 'description', 'version',
- 'parent_series']
+ ['name', 'version', 'displayname', 'summary']
A distroseries is created whent the required field are submitted.
>>> form = {
... 'field.name': 'sane',
... 'field.displayname': 'Sane Name',
- ... 'field.title': 'Ubuntu Sane Name',
... 'field.summary': 'A stable series to introduce fnord.',
- ... 'field.description': 'I am board filling this out',
... 'field.version': '2009.06',
- ... 'field.parent_series': 'ubuntu/hoary',
... 'field.actions.create': 'Create Series',
... }
>>> view = create_initialized_view(ubuntu, '+addseries', form=form)
@@ -270,7 +267,6 @@
>>> yo_form = dict(form)
>>> yo_form['field.name'] = 'island'
>>> yo_form['field.displayname'] = 'Island'
- >>> yo_form['field.title'] = 'YouBuntu Island'
>>> view = create_initialized_view(
... youbuntu, name='+addseries', form=yo_form, principal=yo_driver)
>>> view.errors
@@ -311,9 +307,7 @@
>>> yo_form = dict(form)
>>> del yo_form['field.actions.create']
>>> yo_form['field.displayname'] = 'Mountain'
- >>> yo_form['field.title'] = 'YouBuntu Mountain'
>>> yo_form['field.summary'] = 'Mountain summary'
- >>> yo_form['field.description'] = 'Mountain description'
>>> yo_form['field.actions.change'] = 'Change'
>>> view = create_initialized_view(
... yo_series, name='+edit', form=yo_form, principal=yo_driver)
=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py 2011-03-29 19:05:34 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-04-04 19:56:15 +0000
@@ -10,12 +10,44 @@
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.testing import (
feature_flags,
+ person_logged_in,
set_feature_flag,
TestCaseWithFactory,
)
from lp.testing.views import create_initialized_view
+class TestDistroSeriesAddView(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_submit(self):
+ # When creating a new DistroSeries via DistroSeriesAddView, the title
+ # is set to the same as the displayname (title is, in any case,
+ # deprecated), the description is left empty, and parent_series is
+ # None (DistroSeriesInitializeView takes care of setting that).
+ user = self.factory.makePerson()
+ distribution = self.factory.makeDistribution(owner=user)
+ form = {
+ "field.name": u"polished",
+ "field.version": u"12.04",
+ "field.displayname": u"Polished Polecat",
+ "field.summary": u"Even The Register likes it.",
+ "field.actions.create": u"Add Series",
+ }
+ with person_logged_in(user):
+ create_initialized_view(distribution, "+addseries", form=form)
+ distroseries = distribution.getSeries(u"polished")
+ self.assertEqual(u"polished", distroseries.name)
+ self.assertEqual(u"12.04", distroseries.version)
+ self.assertEqual(u"Polished Polecat", distroseries.displayname)
+ self.assertEqual(u"Polished Polecat", distroseries.title)
+ self.assertEqual(u"Even The Register likes it.", distroseries.summary)
+ self.assertEqual(u"", distroseries.description)
+ self.assertIs(None, distroseries.parent_series)
+ self.assertEqual(user, distroseries.owner)
+
+
class TestDistroSeriesInitializeView(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
=== added file 'lib/lp/registry/templates/distroseries-add.pt'
--- lib/lp/registry/templates/distroseries-add.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/distroseries-add.pt 2011-04-04 19:56:15 +0000
@@ -0,0 +1,21 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad">
+ <body>
+ <div metal:fill-slot="main">
+ <div metal:use-macro="context/@@launchpad_form/form">
+ <p metal:fill-slot="extra_info">
+ This page allows you to add a new distribution series.
+ <a href="/+help/distribution-add-series.html"
+ target="help" class="sprite maybe">
+ <span class="invisible-link">(?)</span>
+ </a>
+ </p>
+ </div>
+ </div>
+ </body>
+</html>
