launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05763
[Merge] lp:~sinzui/launchpad/dsp-vocab-unknown-distro into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/dsp-vocab-unknown-distro into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/dsp-vocab-unknown-distro/+merge/83993
DSP vocabulary must handle the case where the distribution is not yet known.
Launchpad bug: https://bugs.launchpad.net/bugs/889192
Pre-implementation: wallyworld, wgrant
A LocationError is raised when working with +distrotask. An error can
also arise when retargeting a bug from a project to a distro package.
The widget or vocabulary looses the distribution previously set by
the code or the user.
--------------------------------------------------------------------
RULES
* Add tests to cover the known cases of failure
* Update the widget and or vocabulary to preserve the selected
distribution needed to locate the package
* ADDENDUM: The +distrotask looses the DSP field as the data is passed
between the multiform views. setUpFields() is not the right way
to switch the field in conjunction with the feature flag.
QA
With disclosure.dsp_picker.enabled on qastaging
* Visit https://bugs.qastaging.launchpad.net/launchpad/+bug/210943
* Choose the Affects distribution action
* Verify the form renders (no oops)
* Choose a distribution and package, then submit the form
* Verify there is a task for the package shown in the affects table.
* Visit https://bugs.qastaging.launchpad.net/launchpad/+bug/188564
* Expand the launchpad row.
* Choose Ubuntu and select ubuntu/pnm2ppa using the picker
* Submit the form
* Verify the bug was retargeted (no oops)
LINT
lib/lp/app/widgets/launchpadtarget.py
lib/lp/app/widgets/tests/test_launchpadtarget.py
lib/lp/bugs/browser/bugalsoaffects.py
lib/lp/bugs/browser/tests/test_bugalsoaffects.py
lib/lp/bugs/browser/widgets/bugtask.py
lib/lp/registry/vocabularies.py
lib/lp/registry/vocabularies.zcml
lib/lp/registry/tests/test_dsp_vocabularies.py
TEST
./bin/test -vv lp.registry.tests.test_dsp_vocabularies
./bin/test -vv lp.app.widgets.tests.test_launchpadtarget
./bin/test -vv lp.bugs.browser.tests.test_bugalsoaffects
IMPLEMENTATION
Added setDistribution() to the vocabulary so that the callsite can
set the distribution that the user submitted with the form.
lib/lp/registry/vocabularies.py
lib/lp/registry/vocabularies.zcml
lib/lp/registry/tests/test_dsp_vocabularies.py
Updated the LaunchpadTargetWidget to call setDistribution() when the
value is retrieved from the submitted data. Updated the chain of
validators and action methods to not do extra work getting a DSP when
the vocabulary provided a valid one -- this is much faster than the
getSourcePackage() and guessSourcePackage() methods currently used.
lib/lp/app/widgets/launchpadtarget.py
lib/lp/app/widgets/tests/test_launchpadtarget.py
Replaces the setUpFields feature flag hack with a new schema and a
schema property to selected when the feature flag is enabled. Updated
the widget to call setDistribution() when the distribution is retrieved
from the submitted data. Updated the chain of validators and action
methods to not do extra work getting a DSP when the vocabulary provided
a valid one -- this is much faster than the getSourcePackage() and
guessSourcePackage() currently used.
lib/lp/bugs/browser/bugalsoaffects.py
lib/lp/bugs/browser/tests/test_bugalsoaffects.py
lib/lp/bugs/browser/widgets/bugtask.py
--
https://code.launchpad.net/~sinzui/launchpad/dsp-vocab-unknown-distro/+merge/83993
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/dsp-vocab-unknown-distro into lp:launchpad.
=== modified file 'lib/lp/app/widgets/launchpadtarget.py'
--- lib/lp/app/widgets/launchpadtarget.py 2011-11-29 16:29:43 +0000
+++ lib/lp/app/widgets/launchpadtarget.py 2011-11-30 18:42:30 +0000
@@ -135,6 +135,9 @@
" Launchpad" % entered_name)
if self.package_widget.hasInput():
+ if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
+ self.package_widget.vocabulary.setDistribution(
+ distribution)
try:
package_name = self.package_widget.getInputValue()
except ConversionError:
@@ -146,10 +149,8 @@
if package_name is None:
return distribution
try:
- if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
- vocab = self.package_widget.context.vocabulary
- name = package_name.name
- dsp = vocab.getTermByToken(name).value
+ if IDistributionSourcePackage.providedBy(package_name):
+ dsp = package_name
else:
source_name = (
distribution.guessPublishedSourcePackageName(
=== modified file 'lib/lp/app/widgets/tests/test_launchpadtarget.py'
--- lib/lp/app/widgets/tests/test_launchpadtarget.py 2011-11-29 16:28:04 +0000
+++ lib/lp/app/widgets/tests/test_launchpadtarget.py 2011-11-30 18:42:30 +0000
@@ -185,12 +185,30 @@
self.widget.request = LaunchpadTestRequest(form=form)
self.assertFalse(self.widget.hasValidInput())
- def test_getInputValue_package(self):
+ def test_getInputValue_package_spn(self):
# The field value is the package when the package radio button
# is selected and the package sub field has valid input.
self.widget.request = LaunchpadTestRequest(form=self.form)
self.assertEqual(self.package, self.widget.getInputValue())
+ def test_getInputValue_package_spn_dsp_picker_feature_flag(self):
+ # The field value is the package when the package radio button
+ # is selected and the package sub field has valid input.
+ self.widget.request = LaunchpadTestRequest(form=self.form)
+ with FeatureFixture({u"disclosure.dsp_picker.enabled": u"on"}):
+ self.widget.setUpSubWidgets()
+ self.assertEqual(self.package, self.widget.getInputValue())
+
+ def test_getInputValue_package_dsp_dsp_picker_feature_flag(self):
+ # The field value is the package when the package radio button
+ # is selected and the package sub field has valid input.
+ form = self.form
+ form['field.target.package'] = 'fnord/snarf'
+ self.widget.request = LaunchpadTestRequest(form=form)
+ with FeatureFixture({u"disclosure.dsp_picker.enabled": u"on"}):
+ self.widget.setUpSubWidgets()
+ self.assertEqual(self.package, self.widget.getInputValue())
+
def test_getInputValue_package_invalid(self):
# An error is raised when the package is not published in the distro.
form = self.form
=== modified file 'lib/lp/bugs/browser/bugalsoaffects.py'
--- lib/lp/bugs/browser/bugalsoaffects.py 2011-10-21 07:52:46 +0000
+++ lib/lp/bugs/browser/bugalsoaffects.py 2011-11-30 18:42:30 +0000
@@ -17,7 +17,6 @@
Item,
)
from lazr.lifecycle.event import ObjectCreatedEvent
-from lazr.restful.interface import copy_field
from z3c.ptcompat import ViewPageTemplateFile
from zope.app.form.browser import DropdownWidget
from zope.app.form.interfaces import MissingInputError
@@ -287,8 +286,9 @@
else:
task_target = data['distribution']
if data.get('sourcepackagename') is not None:
- task_target = task_target.getSourcePackage(
- data['sourcepackagename'])
+ spn_or_dsp = data['sourcepackagename']
+ if not IDistributionSourcePackage.providedBy(spn_or_dsp):
+ task_target = task_target.getSourcePackage(spn_or_dsp)
self.task_added = self.context.bug.addTask(
getUtility(ILaunchBag).user, task_target)
task_added = self.task_added
@@ -344,10 +344,26 @@
self.next_url = canonical_url(task_added)
+class IAddDistroBugTaskForm(IAddBugTaskForm):
+
+ sourcepackagename = Choice(
+ title=_("Source Package Name"), required=False,
+ description=_("The source package in which the bug occurs. "
+ "Leave blank if you are not sure."),
+ vocabulary='DistributionSourcePackage')
+
+
class DistroBugTaskCreationStep(BugTaskCreationStep):
"""Specialized BugTaskCreationStep for reporting a bug in a distribution.
"""
+ @property
+ def schema(self):
+ if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
+ return IAddDistroBugTaskForm
+ else:
+ return IAddBugTaskForm
+
custom_widget(
'sourcepackagename', BugTaskAlsoAffectsSourcePackageNameWidget)
@@ -356,17 +372,6 @@
label = "Also affects distribution/package"
target_field_names = ('distribution', 'sourcepackagename')
- def setUpFields(self):
- super(DistroBugTaskCreationStep, self).setUpFields()
- if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
- # Replace the default field with a field that uses the better
- # vocabulary.
- self.form_fields = self.form_fields.omit('sourcepackagename')
- new_sourcepackagename = copy_field(
- IAddBugTaskForm['sourcepackagename'],
- vocabulary='DistributionSourcePackage')
- self.form_fields += form.Fields(new_sourcepackagename)
-
@property
def initial_values(self):
"""Return the initial values for the view's fields."""
@@ -441,7 +446,7 @@
distribution.displayname, entered_package,
binary_tracking)
self.setFieldError('sourcepackagename', error)
- else:
+ elif not IDistributionSourcePackage.providedBy(sourcepackagename):
try:
target = distribution
if sourcepackagename:
=== modified file 'lib/lp/bugs/browser/tests/test_bugalsoaffects.py'
--- lib/lp/bugs/browser/tests/test_bugalsoaffects.py 2011-06-08 06:58:01 +0000
+++ lib/lp/bugs/browser/tests/test_bugalsoaffects.py 2011-11-30 18:42:30 +0000
@@ -8,11 +8,9 @@
from canonical.launchpad.testing.pages import get_feedback_messages
from canonical.launchpad.webapp import canonical_url
from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.services.features.testing import FeatureFixture
from lp.soyuz.enums import PackagePublishingStatus
-from lp.testing import (
- person_logged_in,
- TestCaseWithFactory,
- )
+from lp.testing import TestCaseWithFactory
class TestBugAlsoAffectsDistribution(TestCaseWithFactory):
@@ -36,6 +34,39 @@
browser.getControl('Continue').click()
self.assertEqual([], get_feedback_messages(browser.contents))
+ def test_bug_alsoaffects_spn_exists_dsp_picker_feature_flag(self):
+ # If the distribution source package for an spn is official,
+ # there is no error.
+ bug = self.factory.makeBug()
+ distribution, dsp = self.factory.makeDSPCache(
+ distro_name=self.distribution.name, package_name='snarf',
+ make_distro=False)
+ with FeatureFixture({u"disclosure.dsp_picker.enabled": u"on"}):
+ browser = self.getUserBrowser()
+ browser.open(canonical_url(bug))
+ browser.getLink(url='+distrotask').click()
+ browser.getControl('Distribution').value = [distribution.name]
+ browser.getControl('Source Package Name').value = (
+ dsp.sourcepackagename.name)
+ browser.getControl('Continue').click()
+ self.assertEqual([], get_feedback_messages(browser.contents))
+
+ def test_bug_alsoaffects_dsp_exists_dsp_picker_feature_flag(self):
+ # If the distribution source package is official, there is no error.
+ bug = self.factory.makeBug()
+ distribution, dsp = self.factory.makeDSPCache(
+ distro_name=self.distribution.name, package_name='snarf',
+ make_distro=False)
+ with FeatureFixture({u"disclosure.dsp_picker.enabled": u"on"}):
+ browser = self.getUserBrowser()
+ browser.open(canonical_url(bug))
+ browser.getLink(url='+distrotask').click()
+ browser.getControl('Distribution').value = [distribution.name]
+ browser.getControl('Source Package Name').value = (
+ '%s/%s' % (distribution.name, dsp.name))
+ browser.getControl('Continue').click()
+ self.assertEqual([], get_feedback_messages(browser.contents))
+
def test_bug_alsoaffects_spn_not_exists_with_published_binaries(self):
# When the distribution has published binaries, we search both
# source and binary package names.
@@ -43,7 +74,7 @@
distroseries = self.factory.makeDistroSeries(
distribution=self.distribution)
das = self.factory.makeDistroArchSeries(distroseries=distroseries)
- bpph = self.factory.makeBinaryPackagePublishingHistory(
+ self.factory.makeBinaryPackagePublishingHistory(
distroarchseries=das, status=PackagePublishingStatus.PUBLISHED)
self.assertTrue(self.distribution.has_published_binaries)
browser = self.getUserBrowser()
=== modified file 'lib/lp/bugs/browser/widgets/bugtask.py'
--- lib/lp/bugs/browser/widgets/bugtask.py 2011-09-22 20:38:36 +0000
+++ lib/lp/bugs/browser/widgets/bugtask.py 2011-11-30 18:42:30 +0000
@@ -512,6 +512,7 @@
if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
try:
+ self.context.vocabulary.setDistribution(distribution)
source = self.context.vocabulary.getTermByToken(input).value
except NotFoundError:
raise ConversionError(
=== modified file 'lib/lp/registry/tests/test_dsp_vocabularies.py'
--- lib/lp/registry/tests/test_dsp_vocabularies.py 2011-11-28 18:35:27 +0000
+++ lib/lp/registry/tests/test_dsp_vocabularies.py 2011-11-30 18:42:30 +0000
@@ -61,6 +61,12 @@
self.assertEqual(None, vocabulary.distribution)
self.assertEqual(None, vocabulary.dsp)
+ def test_setDistribution(self):
+ new_distro = self.factory.makeDistribution(name='fnord')
+ vocabulary = DistributionSourcePackageVocabulary(None)
+ vocabulary.setDistribution(new_distro)
+ self.assertEqual(new_distro, vocabulary.distribution)
+
def test_getDistributionAndPackageName_distro_and_package(self):
# getDistributionAndPackageName() returns a tuple of distribution
# and package name when the text contains both.
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2011-11-04 06:18:59 +0000
+++ lib/lp/registry/vocabularies.py 2011-11-30 18:42:30 +0000
@@ -2072,6 +2072,9 @@
def __len__(self):
pass
+ def setDistribution(self, distribution):
+ self.distribution = distribution
+
def getDistributionAndPackageName(self, text):
"Return the distribution and package name from the parsed text."
# Match the toTerm() format, but also use it to select a distribution.
=== modified file 'lib/lp/registry/vocabularies.zcml'
--- lib/lp/registry/vocabularies.zcml 2011-11-04 06:18:59 +0000
+++ lib/lp/registry/vocabularies.zcml 2011-11-30 18:42:30 +0000
@@ -472,6 +472,9 @@
<class class="lp.registry.vocabularies.DistributionSourcePackageVocabulary">
<allow interface="canonical.launchpad.webapp.vocabulary.IHugeVocabulary"/>
+ <require
+ permission="zope.Public"
+ attributes="setDistribution"/>
</class>
<class
