launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05054
[Merge] lp:~sinzui/launchpad/valid-targets-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/valid-targets-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/valid-targets-0/+merge/76658
Restrict the target widget to pillars that use Launchpad.
Launchpad bug: https://bugs.launchpad.net/bugs/828838
Pre-implementation: jcsackett
LaunchpadTargetWidget is used for BugTasks and it does not
constrain the choice to pillars that use Launchpad. The widget lists
every distribution in the DB, but very few use Launchpad.
This issue is largely about the ui and the moment. Distributions can
change which bug tracker they use, and the existing bug tasks will
continue to exist. The existing constraint of any distro is correct,
but when I choose to retarget a bug, it should only be to the distros
that currently use Lp. User can use the API to target bugs distros,
and we assume the user knows what he is doing.
Note that a previous attempt to restrict targets to the current state
of what is valid failed because historic data became invalid--you could
not load a bug if one of the bugtasks belonged to a project that did not
use Lp, or the package was never published.
--------------------------------------------------------------------
RULES
* Create vocabularies that constrain the the pillars to those that
use Launchpad, or the current distro so historic data is accepted.
* Subclass the widget to use the appropriate bugtask vocabulary
QA
* Visit https://bugs.qastaging.launchpad.net/ubuntu/+source/linux/+bug/269253
* Expand the bugtask row for the linux project and
verify that gentoo and samsung are not listed
* Expand the samsung bugtask row and
verify gentoo is not listed, but samsung is
* Expand the ubuntu bugtask and
verify that gentoo and samsung are not listed
LINT
lib/lp/app/widgets/launchpadtarget.py
lib/lp/bugs/configure.zcml
lib/lp/bugs/vocabulary.py
lib/lp/bugs/browser/bugtask.py
lib/lp/bugs/browser/widgets/bugtask.py
lib/lp/bugs/browser/widgets/tests/
lib/lp/bugs/browser/widgets/tests/__init__.py
lib/lp/bugs/browser/widgets/tests/test_bugtask.py
lib/lp/bugs/tests/test_vocabulary.py
TEST
./bin/test -vv -t launchpad-target-widget lp.app.widgets.tests.test_doc
./bin/test -vv lp.bug.tests.test_vocabulary
./bin/test -vv lp.bug.browser.widgets.tests.test_bugtask
IMPLEMENTATION
Introduced UsesBugsDistributionVocabulary that know the current set of
legitimate distro bugtargets. It also exempts the distro of the existing
task.
lib/lp/bugs/configure.zcml
lib/lp/bugs/vocabulary.py
lib/lp/bugs/tests/test_vocabulary.py
Extracted the rule to get the the distribution vocabulary in
LaunchpadTargetWidget. Subclassed it to create BugTaskTargetWidget which
uses the new vocabulary.
lib/lp/app/widgets/launchpadtarget.py
lib/lp/bugs/browser/widgets/bugtask.py
lib/lp/bugs/browser/widgets/tests/
lib/lp/bugs/browser/widgets/tests/__init__.py
lib/lp/bugs/browser/widgets/tests/test_bugtask.py
Updated the bugtask edit form to use the BugTaskTargetWidget. And I was
surprised that the forms worked without need of additional tests for edge
cases.
lib/lp/bugs/browser/bugtask.py
--
https://code.launchpad.net/~sinzui/launchpad/valid-targets-0/+merge/76658
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/valid-targets-0 into lp:launchpad.
=== modified file 'lib/lp/app/widgets/launchpadtarget.py'
--- lib/lp/app/widgets/launchpadtarget.py 2011-08-15 13:09:49 +0000
+++ lib/lp/app/widgets/launchpadtarget.py 2011-09-22 21:22:24 +0000
@@ -51,6 +51,9 @@
default_option = "package"
_widgets_set_up = False
+ def getDistributionVocabulary(self):
+ return 'Distribution'
+
def setUpSubWidgets(self):
if self._widgets_set_up:
return
@@ -66,7 +69,7 @@
required=True, vocabulary='Product'),
Choice(
__name__='distribution', title=u"Distribution",
- required=True, vocabulary='Distribution',
+ required=True, vocabulary=self.getDistributionVocabulary(),
default=getUtility(ILaunchpadCelebrities).ubuntu),
Choice(
__name__='package', title=u"Package",
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2011-09-13 05:23:16 +0000
+++ lib/lp/bugs/browser/bugtask.py 2011-09-22 21:22:24 +0000
@@ -182,7 +182,6 @@
IServiceUsage,
)
from lp.app.widgets.itemswidgets import LabeledMultiCheckBoxWidget
-from lp.app.widgets.launchpadtarget import LaunchpadTargetWidget
from lp.app.widgets.popup import PersonPickerWidget
from lp.app.widgets.project import ProjectScopeWidget
from lp.bugs.browser.bug import (
@@ -203,6 +202,7 @@
BugTaskAssigneeWidget,
BugTaskBugWatchWidget,
BugTaskSourcePackageNameWidget,
+ BugTaskTargetWidget,
DBItemDisplayWidget,
NewLineToSpacesWidget,
NominationReviewActionWidget,
@@ -1215,7 +1215,7 @@
# the form.
default_field_names = ['assignee', 'bugwatch', 'importance', 'milestone',
'status']
- custom_widget('target', LaunchpadTargetWidget)
+ custom_widget('target', BugTaskTargetWidget)
custom_widget('sourcepackagename', BugTaskSourcePackageNameWidget)
custom_widget('bugwatch', BugTaskBugWatchWidget)
custom_widget('assignee', BugTaskAssigneeWidget)
=== modified file 'lib/lp/bugs/browser/widgets/bugtask.py'
--- lib/lp/bugs/browser/widgets/bugtask.py 2011-07-29 15:19:08 +0000
+++ lib/lp/bugs/browser/widgets/bugtask.py 2011-09-22 21:22:24 +0000
@@ -10,6 +10,7 @@
"BugTaskAssigneeWidget",
"BugTaskBugWatchWidget",
"BugTaskSourcePackageNameWidget",
+ "BugTaskTargetWidget",
"BugWatchEditForm",
"DBItemDisplayWidget",
"NewLineToSpacesWidget",
@@ -66,11 +67,13 @@
StrippedTextWidget,
URIWidget,
)
+from lp.app.widgets.launchpadtarget import LaunchpadTargetWidget
from lp.bugs.interfaces.bugwatch import (
IBugWatchSet,
NoBugTrackerFound,
UnrecognizedBugTrackerURL,
)
+from lp.bugs.vocabulary import UsesBugsDistributionVocabulary
from lp.registry.interfaces.distribution import IDistributionSet
from lp.services.features import getFeatureFlag
from lp.services.fields import URIField
@@ -473,6 +476,14 @@
contents='\n'.join(rendered_items))
+class BugTaskTargetWidget(LaunchpadTargetWidget):
+
+ def getDistributionVocabulary(self):
+ distro = self.context.context.distribution
+ vocabulary = UsesBugsDistributionVocabulary(distro)
+ return vocabulary
+
+
class BugTaskSourcePackageNameWidget(VocabularyPickerWidget):
"""A widget for associating a bugtask with a SourcePackageName.
=== added directory 'lib/lp/bugs/browser/widgets/tests'
=== added file 'lib/lp/bugs/browser/widgets/tests/__init__.py'
=== added file 'lib/lp/bugs/browser/widgets/tests/test_bugtask.py'
--- lib/lp/bugs/browser/widgets/tests/test_bugtask.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/widgets/tests/test_bugtask.py 2011-09-22 21:22:24 +0000
@@ -0,0 +1,52 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the bugtask widgets."""
+
+__metaclass__ = type
+
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.bugs.browser.widgets.bugtask import BugTaskTargetWidget
+from lp.bugs.interfaces.bugtask import IBugTask
+from lp.testing import TestCaseWithFactory
+
+
+class BugTaskTargetWidgetTestCase(TestCaseWithFactory):
+ """Test that BugTaskTargetWidget behaves as expected."""
+ layer = DatabaseFunctionalLayer
+
+ def getWidget(self, bugtask):
+ field = IBugTask['target']
+ bound_field = field.bind(bugtask)
+ request = LaunchpadTestRequest()
+ return BugTaskTargetWidget(bound_field, request)
+
+ def test_getDistributionVocabulary_with_product_bugtask(self):
+ # The vocabulary does not contain distros that do not use
+ # launchpad to track bugs.
+ distribution = self.factory.makeDistribution()
+ product = self.factory.makeProduct()
+ bugtask = self.factory.makeBugTask(target=product)
+ target_widget = self.getWidget(bugtask)
+ vocabulary = target_widget.getDistributionVocabulary()
+ self.assertEqual(None, vocabulary.distribution)
+ self.assertFalse(
+ distribution in vocabulary,
+ "Vocabulary contains distros that do not use Launchpad Bugs.")
+
+ def test_getDistributionVocabulary_with_distribution_bugtask(self):
+ # The vocabulary does not contain distros that do not use
+ # launchpad to track bugs.
+ distribution = self.factory.makeDistribution()
+ other_distribution = self.factory.makeDistribution()
+ bugtask = self.factory.makeBugTask(target=distribution)
+ target_widget = self.getWidget(bugtask)
+ vocabulary = target_widget.getDistributionVocabulary()
+ self.assertEqual(distribution, vocabulary.distribution)
+ self.assertTrue(
+ distribution in vocabulary,
+ "Vocabulary missing context distribution.")
+ self.assertFalse(
+ other_distribution in vocabulary,
+ "Vocabulary contains distros that do not use Launchpad Bugs.")
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-09-19 08:35:03 +0000
+++ lib/lp/bugs/configure.zcml 2011-09-22 21:22:24 +0000
@@ -1187,4 +1187,11 @@
<webservice:register module="lp.bugs.errors" />
<webservice:register module="lp.bugs.interfaces.webservice" />
+ <securedutility
+ name="UsesBugsDistribution"
+ component="lp.bugs.vocabulary.UsesBugsDistributionVocabulary"
+ provides="zope.schema.interfaces.IVocabularyFactory"
+ >
+ <allow interface="zope.schema.interfaces.IVocabularyFactory"/>
+ </securedutility>
</configure>
=== added file 'lib/lp/bugs/tests/test_vocabulary.py'
--- lib/lp/bugs/tests/test_vocabulary.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/tests/test_vocabulary.py 2011-09-22 21:22:24 +0000
@@ -0,0 +1,69 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the bug domain vocabularies."""
+
+__metaclass__ = type
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.bugs.vocabulary import UsesBugsDistributionVocabulary
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+
+
+class UsesBugsDistributionVocabularyTestCase(TestCaseWithFactory):
+ """Test that the vocabulary behaves as expected."""
+ layer = DatabaseFunctionalLayer
+
+ def test_init_with_distribution(self):
+ # When the context is adaptable to IDistribution, the distribution
+ # property is the distribution.
+ distribution = self.factory.makeDistribution()
+ vocabulary = UsesBugsDistributionVocabulary(distribution)
+ self.assertEqual(distribution, vocabulary.context)
+ self.assertEqual(distribution, vocabulary.distribution)
+
+ def test_init_without_distribution(self):
+ # When the context is not adaptable to IDistribution, the
+ # distribution property is None
+ thing = self.factory.makeProduct()
+ vocabulary = UsesBugsDistributionVocabulary(thing)
+ self.assertEqual(thing, vocabulary.context)
+ self.assertEqual(None, vocabulary.distribution)
+
+ def test_contains_distros_that_use_bugs(self):
+ # The vocabulary contains distributions that also use
+ # Launchpad to track bugs.
+ distro_less_bugs = self.factory.makeDistribution()
+ distro_uses_bugs = self.factory.makeDistribution()
+ with person_logged_in(distro_uses_bugs.owner):
+ distro_uses_bugs.official_malone = True
+ vocabulary = UsesBugsDistributionVocabulary()
+ self.assertFalse(
+ distro_less_bugs in vocabulary,
+ "Vocabulary contains distros that do not use Launchpad Bugs.")
+ self.assertTrue(
+ distro_uses_bugs in vocabulary,
+ "Vocabulary missing distros that use Launchpad Bugs.")
+
+ def test_contains_context_distro(self):
+ # The vocabulary contains the context distro even it it does not
+ # use Launchpad to track bugs. The distro may have tracked bugs
+ # in the past so it is a legitimate choise for historic data.
+ distro_less_bugs = self.factory.makeDistribution()
+ vocabulary = UsesBugsDistributionVocabulary(distro_less_bugs)
+ self.assertFalse(distro_less_bugs.official_malone)
+ self.assertTrue(
+ distro_less_bugs in vocabulary,
+ "Vocabulary missing context distro.")
+
+ def test_contains_missing_context(self):
+ # The vocabulary contains does not contain the context if the
+ # context is not adaptable to a distribution.
+ thing = self.factory.makeProduct()
+ vocabulary = UsesBugsDistributionVocabulary(thing)
+ self.assertFalse(
+ thing in vocabulary,
+ "Vocabulary contains a non-distribution.")
=== added file 'lib/lp/bugs/vocabulary.py'
--- lib/lp/bugs/vocabulary.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/vocabulary.py 2011-09-22 21:22:24 +0000
@@ -0,0 +1,38 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Bug domain vocabularies"""
+
+__metaclass__ = type
+__all__ = [
+ 'UsesBugsDistributionVocabulary',
+ ]
+
+from sqlobject import OR
+
+from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.vocabularies import DistributionVocabulary
+
+
+class UsesBugsDistributionVocabulary(DistributionVocabulary):
+ """Distributions that use Launchpad to track bugs.
+
+ If the context is a distribution, it is always included in the
+ vocabulary. Historic data is not invalidated if a distro stops
+ using Launchpad to track bugs. This vocabulary offers the correct
+ choices of distributions at this moment.
+ """
+
+ def __init__(self, context=None):
+ super(UsesBugsDistributionVocabulary, self).__init__(context=context)
+ self.distribution = IDistribution(self.context, None)
+
+ @property
+ def _filter(self):
+ if self.distribution is None:
+ distro_id = 0
+ else:
+ distro_id = self.distribution.id
+ return OR(
+ self._table.q.official_malone == True,
+ self._table.id == distro_id)
