launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02307
[Merge] lp:~allenap/launchpad/sub-search-ui-bug-656823-4 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/sub-search-ui-bug-656823-4 into lp:launchpad with lp:~allenap/launchpad/sub-search-ui-bug-656823-3 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/launchpad/sub-search-ui-bug-656823-4/+merge/45824
The main purpose of this branch is to provide an edit view for bug
subscription filters. However, to pull this off several things also
got done:
- I updated the IBugSubscriptionFilter field descriptions to work
better in the web UI. I think they make sense for the web service
API doc too.
- Add a new widget, BugTagsFrozenSetWidget, that subclasses
BugTagsWidget. I made some changes to the latter to make the
subclassing a bit more elegant.
- I moved canonical.widgets.bug to lp.bugs.browser.widgets.bug. I also
moved the widget registration ZCML to the lp.bugs.brower.widgets
package.
- Added some rudimentary "(edit)" links to the structural subscription
overview page. They are there as much to make manual testing and UI
review easier (though I doubt this branch warrants a UI review as it
stands).
- Some imports and lint needed fixing, but I've stuffed those into a
later branch because this branch is noisy enough.
--
https://code.launchpad.net/~allenap/launchpad/sub-search-ui-bug-656823-4/+merge/45824
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/sub-search-ui-bug-656823-4 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2010-12-21 15:28:54 +0000
+++ lib/lp/bugs/browser/bug.py 2011-01-11 10:08:37 +0000
@@ -86,7 +86,6 @@
ICanonicalUrlData,
ILaunchBag,
)
-from canonical.widgets.bug import BugTagsWidget
from canonical.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
from canonical.widgets.project import ProjectScopeWidget
from lp.app.browser.launchpadform import (
@@ -97,6 +96,7 @@
)
from lp.app.browser.stringformatter import FormattersAPI
from lp.app.errors import NotFoundError
+from lp.bugs.browser.widgets.bug import BugTagsWidget
from lp.bugs.interfaces.bug import (
IBug,
IBugSet,
=== modified file 'lib/lp/bugs/browser/bugsubscriptionfilter.py'
--- lib/lp/bugs/browser/bugsubscriptionfilter.py 2011-01-11 10:08:27 +0000
+++ lib/lp/bugs/browser/bugsubscriptionfilter.py 2011-01-11 10:08:37 +0000
@@ -9,8 +9,21 @@
]
+from zope.app.form.browser import TextWidget
+
from canonical.launchpad.helpers import english_list
-from canonical.launchpad.webapp.publisher import LaunchpadView
+from canonical.launchpad.webapp.publisher import (
+ canonical_url,
+ LaunchpadView,
+ )
+from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget
+from lp.app.browser.launchpadform import (
+ action,
+ custom_widget,
+ LaunchpadEditFormView,
+ )
+from lp.bugs.browser.widgets.bug import BugTagsFrozenSetWidget
+from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
class BugSubscriptionFilterView(LaunchpadView):
@@ -59,3 +72,35 @@
sorted(tags), conjunction=(
u"and" if self.context.find_all_tags else u"or")))
return conditions
+
+
+class BugSubscriptionFilterEditView(LaunchpadEditFormView):
+ """Edit view for `IBugSubscriptionFilter`."""
+
+ page_title = u"Edit filter"
+ schema = IBugSubscriptionFilter
+ field_names = (
+ "description",
+ "statuses",
+ "importances",
+ "tags",
+ "find_all_tags",
+ )
+
+ custom_widget("description", TextWidget, displayWidth=50)
+ custom_widget("statuses", LabeledMultiCheckBoxWidget)
+ custom_widget("importances", LabeledMultiCheckBoxWidget)
+ custom_widget("tags", BugTagsFrozenSetWidget, displayWidth=35)
+
+ @action("Update", name="update")
+ def update_action(self, action, data):
+ """Update the bug filter with the form data."""
+ self.updateContextFromData(data)
+
+ @property
+ def next_url(self):
+ """Return to the user's structural subscriptions page."""
+ return canonical_url(
+ self.user, view_name="+structural-subscriptions")
+
+ cancel_url = next_url
=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py 2010-12-02 16:14:30 +0000
+++ lib/lp/bugs/browser/bugtarget.py 2011-01-11 10:08:37 +0000
@@ -58,9 +58,7 @@
FeedsMixin,
)
from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
-from canonical.launchpad.interfaces.launchpad import (
- ILaunchpadCelebrities,
- )
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.searchbuilder import any
from canonical.launchpad.validators.name import valid_name_pattern
from canonical.launchpad.webapp import (
@@ -74,10 +72,6 @@
from canonical.launchpad.webapp.interfaces import ILaunchBag
from canonical.launchpad.webapp.menu import structured
from canonical.launchpad.webapp.publisher import HTTP_MOVED_PERMANENTLY
-from canonical.widgets.bug import (
- BugTagsWidget,
- LargeBugTagsWidget,
- )
from canonical.widgets.bugtask import NewLineToSpacesWidget
from canonical.widgets.product import (
GhostCheckBoxWidget,
@@ -103,6 +97,10 @@
)
from lp.bugs.browser.bugrole import BugRoleMixin
from lp.bugs.browser.bugtask import BugTaskSearchListingView
+from lp.bugs.browser.widgets.bug import (
+ BugTagsWidget,
+ LargeBugTagsWidget,
+ )
from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
from lp.bugs.interfaces.bug import (
CreateBugParams,
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2010-12-22 03:50:13 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-01-11 10:08:37 +0000
@@ -164,7 +164,6 @@
from canonical.launchpad.webapp.menu import structured
from canonical.lazr.interfaces import IObjectPrivacy
from canonical.lazr.utils import smartquote
-from canonical.widgets.bug import BugTagsWidget
from canonical.widgets.bugtask import (
AssigneeDisplayWidget,
BugTaskAssigneeWidget,
@@ -208,6 +207,7 @@
build_comments_from_chunks,
group_comments_with_activity,
)
+from lp.bugs.browser.widgets.bug import BugTagsWidget
from lp.bugs.interfaces.bug import (
IBug,
IBugSet,
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2011-01-11 10:08:27 +0000
+++ lib/lp/bugs/browser/configure.zcml 2011-01-11 10:08:37 +0000
@@ -1189,6 +1189,15 @@
template="../templates/bug-subscription-filter.pt"
permission="launchpad.View"
name="+definition" />
+ <browser:page
+ for="lp.bugs.interfaces.bugsubscriptionfilter.IBugSubscriptionFilter"
+ class=".bugsubscriptionfilter.BugSubscriptionFilterEditView"
+ template="../templates/bug-subscription-filter-edit.pt"
+ permission="launchpad.Edit"
+ name="+edit" />
</facet>
+ <!-- Widgets -->
+ <include package=".widgets"/>
+
</configure>
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py'
--- lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-01-11 10:08:27 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-01-11 10:08:37 +0000
@@ -31,10 +31,12 @@
from lp.testing import (
normalize_whitespace,
person_logged_in,
+ login_person,
TestCaseWithFactory,
ws_object,
)
from lp.testing.views import create_initialized_view
+from lp.bugs.publisher import BugsLayer
class TestBugSubscriptionFilterBase:
@@ -306,7 +308,7 @@
# the absense of conditions.
self.assertRender(
u"This filter does not allow mail through.",
- u"There are no filter conditions!")
+ u"There are no filter conditions! (edit)")
def test_render_with_no_description_and_conditions(self):
# If conditions are set but no description, the rendered description
@@ -325,7 +327,7 @@
self.subscription_filter.tags = [u"foo", u"bar"]
self.assertRender(
u"This filter allows mail through when:",
- u" and ".join(self.view.conditions))
+ u" and ".join(self.view.conditions) + u" (edit)")
def test_render_with_description_and_no_conditions(self):
# If a description is set it appears in the content of the dt tag,
@@ -334,7 +336,7 @@
self.subscription_filter.description = u"The Wait"
self.assertRender(
u"\u201cThe Wait\u201d does not allow mail through.",
- u"There are no filter conditions!")
+ u"There are no filter conditions! (edit)")
def test_render_with_description_and_conditions(self):
# If a description is set it appears in the content of the dt tag,
@@ -344,4 +346,53 @@
self.subscription_filter.tags = [u"foo"]
self.assertRender(
u"\u201cThe Wait\u201d allows mail through when:",
- u" and ".join(self.view.conditions))
+ u" and ".join(self.view.conditions) + u" (edit)")
+
+
+class TestBugSubscriptionFilterEditView(
+ TestBugSubscriptionFilterBase, TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_view_properties(self):
+ # The cancel url and next url will both point to the user's structural
+ # subscription overview page.
+ login_person(self.owner)
+ view = create_initialized_view(
+ self.subscription_filter, name="+edit",
+ layer=BugsLayer, principal=self.owner)
+ self.assertEqual([], view.errors)
+ path = "/~%s/+structural-subscriptions" % self.owner.name
+ self.assertEqual(path, urlparse(view.cancel_url).path)
+ self.assertEqual(path, urlparse(view.next_url).path)
+
+ def test_edit(self):
+ # The filter can be updated by using the update action.
+ form = {
+ "field.description": "New description",
+ "field.statuses": ["NEW", "INCOMPLETE"],
+ "field.importances": ["LOW", "MEDIUM"],
+ "field.tags": u"foo bar",
+ "field.find_all_tags": "on",
+ "field.actions.update": "Update",
+ }
+ with person_logged_in(self.owner):
+ view = create_initialized_view(
+ self.subscription_filter, name="+edit",
+ form=form, layer=BugsLayer, principal=self.owner)
+ self.assertEqual([], view.errors)
+ # The subscription filter has been updated.
+ self.assertEqual(
+ u"New description",
+ self.subscription_filter.description)
+ self.assertEqual(
+ frozenset([BugTaskStatus.NEW, BugTaskStatus.INCOMPLETE]),
+ self.subscription_filter.statuses)
+ self.assertEqual(
+ frozenset([BugTaskImportance.LOW, BugTaskImportance.MEDIUM]),
+ self.subscription_filter.importances)
+ self.assertEqual(
+ frozenset([u"foo", u"bar"]),
+ self.subscription_filter.tags)
+ self.assertTrue(
+ self.subscription_filter.find_all_tags)
=== added directory 'lib/lp/bugs/browser/widgets'
=== added file 'lib/lp/bugs/browser/widgets/__init__.py'
=== renamed file 'lib/canonical/widgets/bug.py' => 'lib/lp/bugs/browser/widgets/bug.py'
--- lib/canonical/widgets/bug.py 2010-10-03 15:30:06 +0000
+++ lib/lp/bugs/browser/widgets/bug.py 2011-01-11 10:08:37 +0000
@@ -2,6 +2,12 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
+__all__ = [
+ "BugTagsFrozenSetWidget",
+ "BugTagsWidget",
+ "BugWidget",
+ "LargeBugTagsWidget",
+ ]
import re
@@ -18,6 +24,7 @@
class BugWidget(IntWidget):
"""A widget for displaying a field that is bound to an IBug."""
+
def _toFormValue(self, value):
"""See zope.app.form.widget.SimpleInputWidget."""
if value == self.context.missing_value:
@@ -44,23 +51,46 @@
class BugTagsWidgetBase:
"""Base class for bug tags widgets."""
+ def _tagsFromFieldValue(self, tags):
+ """Package up the tags for display.
+
+ Override this to provide custom ordering for example.
+
+ :return: `None` if there are no tags, else an iterable of tags.
+ """
+ if tags is None or len(tags) == 0:
+ return None
+ else:
+ return tags
+
+ def _tagsToFieldValue(self, tags):
+ """Package up the tags for the field.
+
+ :param tags: `None` if the submitted data was missing, otherwise an
+ iterable of tags.
+ """
+ if tags is None:
+ return []
+ else:
+ return sorted(set(tags))
+
def _toFormValue(self, value):
"""Convert a list of strings to a single, space separated, string."""
- if value:
- return u' '.join(value)
+ tags = self._tagsFromFieldValue(value)
+ if tags is None:
+ return self._missing
else:
- return self._missing
+ return u" ".join(tags)
def _toFieldValue(self, input):
"""Convert a space separated string to a list of strings."""
input = input.strip()
if input == self._missing:
- return []
+ return self._tagsToFieldValue(None)
else:
- tags = set(tag.lower()
- for tag in re.split(r'[,\s]+', input)
- if len(tag) != 0)
- return sorted(tags)
+ return self._tagsToFieldValue(
+ tag.lower() for tag in re.split(r'[,\s]+', input)
+ if len(tag) != 0)
def getInputValue(self):
try:
@@ -88,8 +118,9 @@
def _getInputValue(self):
raise NotImplementedError('_getInputValue must be overloaded')
+
class BugTagsWidget(BugTagsWidgetBase, TextWidget):
- """A widget for editing bug tags."""
+ """A widget for editing bug tags in a `List` field."""
def __init__(self, field, value_type, request):
# We don't use value_type.
@@ -99,8 +130,29 @@
return TextWidget.getInputValue(self)
+class BugTagsFrozenSetWidget(BugTagsWidget):
+ """A widget for editing bug tags in a `FrozenSet` field."""
+
+ def _tagsFromFieldValue(self, tags):
+ """Order the tags for display.
+
+ The field value is assumed to be unordered.
+ """
+ if tags is None or len(tags) == 0:
+ return None
+ else:
+ return sorted(tags)
+
+ def _tagsToFieldValue(self, tags):
+ """Return a `frozenset` of tags."""
+ if tags is None:
+ return frozenset()
+ else:
+ return frozenset(tags)
+
+
class LargeBugTagsWidget(BugTagsWidgetBase, TextAreaWidget):
- """A large widget for editing bug tags."""
+ """A large widget for editing bug tags in a `List` field."""
def __init__(self, field, value_type, request):
# We don't use value_type.
=== added file 'lib/lp/bugs/browser/widgets/configure.zcml'
--- lib/lp/bugs/browser/widgets/configure.zcml 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/widgets/configure.zcml 2011-01-11 10:08:37 +0000
@@ -0,0 +1,19 @@
+<!-- Copyright 2011 Canonical Ltd. This software is licensed under the
+ GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<configure
+ xmlns="http://namespaces.zope.org/zope";
+ xmlns:browser="http://namespaces.zope.org/browser";
+ xmlns:i18n="http://namespaces.zope.org/i18n";
+ i18n_domain="launchpad">
+
+ <view
+ type="zope.publisher.interfaces.browser.IBrowserRequest"
+ for="lp.services.fields.IBugField"
+ provides="zope.app.form.interfaces.IInputWidget"
+ factory="lp.bugs.browser.widgets.bug.BugWidget"
+ permission="zope.Public"
+ />
+
+</configure>
=== modified file 'lib/lp/bugs/doc/bug-tags.txt'
--- lib/lp/bugs/doc/bug-tags.txt 2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/doc/bug-tags.txt 2011-01-11 10:08:37 +0000
@@ -68,7 +68,7 @@
use BugTagsWidget.
>>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
- >>> from canonical.widgets.bug import BugTagsWidget
+ >>> from lp.bugs.browser.widgets.bug import BugTagsWidget
>>> from lp.bugs.interfaces.bug import IBug
>>> bug_tags_field = IBug['tags'].bind(bug_one)
>>> tag_field = bug_tags_field.value_type
@@ -158,6 +158,49 @@
[u'bar', u'foo']
+=== Bug Tags Widget for Frozen Sets ===
+
+A variant of `BugTagsWidget` exists for when tags are stored in a
+`FrozenSet` field.
+
+ >>> from lp.bugs.browser.widgets.bug import BugTagsFrozenSetWidget
+
+Field-manipulation is not going to be examined here, and the widget
+does not care what type the field is otherwise, so the field from
+earlier can be used again.
+
+ >>> tags_frozen_set_widget = BugTagsFrozenSetWidget(
+ ... bug_tags_field, tag_field, request)
+
+_tagsFromFieldValue() converts tags from the field value to tags for
+display. The absense of tags causes it to return None:
+
+ >>> print tags_frozen_set_widget._tagsFromFieldValue(None)
+ None
+ >>> print tags_frozen_set_widget._tagsFromFieldValue(frozenset())
+ None
+
+Tags are ordered before returning:
+
+ >>> tags_frozen_set_widget._tagsFromFieldValue(
+ ... frozenset([5, 4, 1, 12]))
+ [1, 4, 5, 12]
+
+_tagsToFieldValue() converts the tags entered in the form into a value
+suitable for the field. In the absense of tags it returns an empty
+frozenset():
+
+ >>> tags_frozen_set_widget._tagsToFieldValue(None)
+ frozenset([])
+ >>> tags_frozen_set_widget._tagsToFieldValue([])
+ frozenset([])
+
+Otherwise it returns a `frozenset` of the tags given:
+
+ >>> tags_frozen_set_widget._tagsToFieldValue([u"foo", u"bar"])
+ frozenset([u'foo', u'bar'])
+
+
=== Large and Small Bug Tags Widget ===
A regular BugTagsWidget is rendered as an <input> tag,
@@ -167,7 +210,7 @@
A LargeBugTagsWidget is rendered as a <textarea>,
- >>> from canonical.widgets.bug import LargeBugTagsWidget
+ >>> from lp.bugs.browser.widgets.bug import LargeBugTagsWidget
>>> large_text_widget = LargeBugTagsWidget(
... bug_tags_field, tag_field, request)
>>> print large_text_widget()
=== modified file 'lib/lp/bugs/doc/bugwidget.txt'
--- lib/lp/bugs/doc/bugwidget.txt 2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/doc/bugwidget.txt 2011-01-11 10:08:37 +0000
@@ -3,7 +3,7 @@
The BugWidget converts string bug ids to the corresponding bug object.
>>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
- >>> from canonical.widgets.bug import BugWidget
+ >>> from lp.bugs.browser.widgets.bug import BugWidget
>>> from lp.services.fields import BugField
>>> bug_field = BugField(__name__='bug', title=u'Bug')
>>> request = LaunchpadTestRequest(form={'field.bug': '1'})
=== modified file 'lib/lp/bugs/interfaces/bugsubscriptionfilter.py'
--- lib/lp/bugs/interfaces/bugsubscriptionfilter.py 2010-12-21 09:34:17 +0000
+++ lib/lp/bugs/interfaces/bugsubscriptionfilter.py 2011-01-11 10:08:37 +0000
@@ -48,37 +48,37 @@
find_all_tags = exported(
Bool(
- title=_("All given tags must be found, or any."),
+ title=_("All given tags must be found"),
required=True, default=False))
include_any_tags = Bool(
- title=_("Include any tags."),
+ title=_("Include any tags"),
required=True, default=False)
exclude_any_tags = Bool(
- title=_("Exclude all tags."),
+ title=_("Exclude all tags"),
required=True, default=False)
description = exported(
Text(
- title=_("Description of this filter."),
+ title=_("A short description of this filter"),
required=False))
statuses = exported(
FrozenSet(
- title=_("The statuses to filter on."),
+ title=_("The statuses interested in (empty for all)"),
required=True, default=frozenset(),
value_type=Choice(
title=_('Status'), vocabulary=BugTaskStatus)))
importances = exported(
FrozenSet(
- title=_("The importances to filter on."),
+ title=_("The importances interested in (empty for all)"),
required=True, default=frozenset(),
value_type=Choice(
title=_('Importance'), vocabulary=BugTaskImportance)))
tags = exported(
FrozenSet(
- title=_("The tags to filter on."),
+ title=_("The tags interested in"),
required=True, default=frozenset(),
value_type=SearchTag()))
=== added file 'lib/lp/bugs/templates/bug-subscription-filter-edit.pt'
--- lib/lp/bugs/templates/bug-subscription-filter-edit.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/templates/bug-subscription-filter-edit.pt 2011-01-11 10:08:37 +0000
@@ -0,0 +1,14 @@
+<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";
+ xml:lang="en" lang="en" dir="ltr"
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="malone">
+ <body>
+ <div metal:fill-slot="main">
+ <div metal:use-macro="context/@@launchpad_form/form" />
+ </div>
+ </body>
+</html>
=== modified file 'lib/lp/bugs/templates/bug-subscription-filter.pt'
--- lib/lp/bugs/templates/bug-subscription-filter.pt 2011-01-11 10:08:27 +0000
+++ lib/lp/bugs/templates/bug-subscription-filter.pt 2011-01-11 10:08:37 +0000
@@ -27,8 +27,10 @@
<b>and</b> <br />
</tal:conjunction>
</tal:conditions>
+ <br /><a tal:attributes="href context/fmt:url/+edit">(edit)</a>
</dd>
<dd tal:condition="not:conditions">
There are no filter conditions!
+ <br /><a tal:attributes="href context/fmt:url/+edit">(edit)</a>
</dd>
</dl>
=== modified file 'lib/lp/services/fields/configure.zcml'
--- lib/lp/services/fields/configure.zcml 2010-08-12 18:11:47 +0000
+++ lib/lp/services/fields/configure.zcml 2011-01-11 10:08:37 +0000
@@ -52,14 +52,6 @@
<view
type="zope.publisher.interfaces.browser.IBrowserRequest"
- for="lp.services.fields.IBugField"
- provides="zope.app.form.interfaces.IInputWidget"
- factory="canonical.widgets.bug.BugWidget"
- permission="zope.Public"
- />
-
- <view
- type="zope.publisher.interfaces.browser.IBrowserRequest"
for="lp.services.fields.IStrippedTextLine"
provides="zope.app.form.interfaces.IInputWidget"
factory="canonical.widgets.textwidgets.StrippedTextWidget"
