launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11988
[Merge] lp:~wallyworld/launchpad/branch-move_project-402915 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/branch-move_project-402915 into lp:launchpad.
Requested reviews:
Curtis Hovey (sinzui)
Related bugs:
Bug #402915 in Launchpad itself: "Can no longer move a branch to another project using the UI"
https://bugs.launchpad.net/launchpad/+bug/402915
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/branch-move_project-402915/+merge/124333
== Implementation ==
Create a new BranchTargetWidget and use it for changing the target of a non package branch.
The widget is similar to the current target widget used for bugtasks. There are two radio buttons, one for personal (junk) branches, the other for product branches. When the product branch button is selected, a product chooser popup is used to select a product.
When the target is changed, a notification message tells the user about it. The widget is not rendered for package branches, only junk and product branches.
== Tests ==
Add tests to TestBranchEditView
Add new tests for the branch target widget.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/code/browser/branch.py
lib/lp/code/browser/tests/test_branch.py
lib/lp/code/browser/widgets/branchtarget.py
lib/lp/code/browser/widgets/templates/
lib/lp/code/browser/widgets/templates/branch-target.pt
lib/lp/code/browser/widgets/tests/test_branchtargetwidget.py
--
https://code.launchpad.net/~wallyworld/launchpad/branch-move_project-402915/+merge/124333
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2012-08-29 00:28:19 +0000
+++ lib/lp/code/browser/branch.py 2012-09-14 04:09:21 +0000
@@ -44,6 +44,7 @@
copy_field,
use_template,
)
+from lazr.restful.fields import Reference
from lazr.restful.utils import smartquote
from lazr.uri import URI
import pytz
@@ -100,6 +101,7 @@
from lp.code.browser.branchref import BranchRef
from lp.code.browser.decorations import DecoratedBranch
from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
+from lp.code.browser.widgets.branchtarget import BranchTargetWidget
from lp.code.enums import (
BranchType,
CodeImportResultStatus,
@@ -121,6 +123,7 @@
)
from lp.code.interfaces.branchcollection import IAllBranches
from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
+from lp.code.interfaces.branchtarget import IBranchTarget
from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
from lp.registry.enums import InformationType
from lp.registry.interfaces.person import IPersonSet
@@ -790,6 +793,12 @@
vocabulary=InformationTypeVocabulary(types=info_types))
reviewer = copy_field(IBranch['reviewer'], required=True)
owner = copy_field(IBranch['owner'], readonly=False)
+ target = Reference(
+ title=_('Branch target'), required=True,
+ schema=IBranchTarget,
+ description=_('The project (if any) this branch pertains to. '
+ 'If no project is specified, then it is a personal '
+ 'branch'))
return BranchEditSchema
@property
@@ -824,6 +833,23 @@
self.request.response.addNotification(
"The branch owner has been changed to %s (%s)"
% (new_owner.displayname, new_owner.name))
+ if 'target' in data:
+ target = data.pop('target')
+ if target == '+junk':
+ target = None
+ if (target is None and self.context.target is not None
+ or target is not None and self.context.target is None
+ or target != self.context.target):
+ self.context.setTarget(self.user, project=target)
+ changed = True
+ if target:
+ self.request.response.addNotification(
+ "The branch target has been changed to %s (%s)"
+ % (target.displayname, target.name))
+ else:
+ self.request.response.addNotification(
+ "This branch is now a personal branch for %s"
+ % self.context.owner.displayname)
if 'private' in data:
# Read only for display.
data.pop('private')
@@ -1080,10 +1106,15 @@
@property
def field_names(self):
- return [
- 'owner', 'name', 'information_type', 'url', 'description',
- 'lifecycle_status']
+ field_names = ['owner', 'name']
+ if not self.context.sourcepackagename:
+ field_names.append('target')
+ field_names.extend([
+ 'information_type', 'url', 'description',
+ 'lifecycle_status'])
+ return field_names
+ custom_widget('target', BranchTargetWidget)
custom_widget('lifecycle_status', LaunchpadRadioWidgetWithDescription)
custom_widget('information_type', LaunchpadRadioWidgetWithDescription)
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2012-09-06 00:01:38 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2012-09-14 04:09:21 +0000
@@ -958,6 +958,67 @@
with person_logged_in(person):
self.assertEquals(team, branch.owner)
+ def test_branch_target_widget_renders_junk(self):
+ # The branch target widget renders correctly for a junk branch.
+ person = self.factory.makePerson()
+ branch = self.factory.makePersonalBranch(owner=person)
+ login_person(person)
+ view = create_initialized_view(branch, name='+edit')
+ self.assertEqual('personal', view.widgets['target'].default_option)
+
+ def test_branch_target_widget_renders_product(self):
+ # The branch target widget renders correctly for a product branch.
+ person = self.factory.makePerson()
+ product = self.factory.makeLegacyProduct()
+ branch = self.factory.makeProductBranch(product=product, owner=person)
+ login_person(person)
+ view = create_initialized_view(branch, name='+edit')
+ self.assertEqual('product', view.widgets['target'].default_option)
+ self.assertEqual(
+ product.name, view.widgets['target'].product_widget.selected_value)
+
+ def test_no_branch_target_widget_for_source_package_branch(self):
+ # The branch target widget is not rendered for a package branch.
+ person = self.factory.makePerson()
+ branch = self.factory.makePackageBranch(owner=person)
+ login_person(person)
+ view = create_initialized_view(branch, name='+edit')
+ self.assertFalse(view.widgets.get('target'))
+
+ def test_branch_target_widget_saves_junk(self):
+ # The branch target widget can retarget to a junk branch.
+ person = self.factory.makePerson()
+ product = self.factory.makeLegacyProduct()
+ branch = self.factory.makeProductBranch(product=product, owner=person)
+ login_person(person)
+ form = {
+ 'field.target': 'personal',
+ 'field.actions.change': 'Change Branch',
+ }
+ view = create_initialized_view(branch, name='+edit', form=form)
+ self.assertEqual(person, branch.target.context)
+ self.assertEqual(
+ 'This branch is now a personal branch for %s' % person.displayname,
+ view.request.response.notifications[0].message)
+
+ def test_branch_target_widget_saves_product(self):
+ # The branch target widget can retarget to a product branch.
+ person = self.factory.makePerson()
+ branch = self.factory.makePersonalBranch(owner=person)
+ product = self.factory.makeProduct()
+ login_person(person)
+ form = {
+ 'field.target': 'product',
+ 'field.target.product': product.name,
+ 'field.actions.change': 'Change Branch',
+ }
+ view = create_initialized_view(branch, name='+edit', form=form)
+ self.assertEqual(product, branch.target.context)
+ self.assertEqual(
+ 'The branch target has been changed to %s (%s)'
+ % (product.displayname, product.name),
+ view.request.response.notifications[0].message)
+
def test_information_type_in_ui(self):
# The information_type of a branch can be changed via the UI by an
# authorised user.
=== added file 'lib/lp/code/browser/widgets/branchtarget.py'
--- lib/lp/code/browser/widgets/branchtarget.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/widgets/branchtarget.py 2012-09-14 04:09:21 +0000
@@ -0,0 +1,135 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = [
+ 'BranchTargetWidget',
+ ]
+
+from z3c.ptcompat import ViewPageTemplateFile
+from zope.app.form import (
+ InputWidget,
+ )
+from zope.app.form.browser.widget import (
+ BrowserWidget,
+ renderElement,
+ )
+from zope.app.form.interfaces import (
+ ConversionError,
+ IInputWidget,
+ InputErrors,
+ MissingInputError,
+ )
+from zope.app.form.utility import setUpWidget
+from zope.interface import implements
+from zope.schema import Choice
+
+from lp.app.errors import (
+ UnexpectedFormData,
+ )
+from lp.app.validators import LaunchpadValidationError
+from lp.code.interfaces.branchtarget import IBranchTarget
+from lp.registry.interfaces.person import IPerson
+from lp.registry.interfaces.product import IProduct
+from lp.services.webapp.interfaces import (
+ IAlwaysSubmittedWidget,
+ IMultiLineWidgetLayout,
+ )
+
+
+class BranchTargetWidget(BrowserWidget, InputWidget):
+ """Widget for selecting a personal (+junk) or product branch target."""
+
+ implements(IAlwaysSubmittedWidget, IMultiLineWidgetLayout, IInputWidget)
+
+ template = ViewPageTemplateFile('templates/branch-target.pt')
+ default_option = "product"
+ _widgets_set_up = False
+
+ def setUpSubWidgets(self):
+ if self._widgets_set_up:
+ return
+ fields = [
+ Choice(
+ __name__='product', title=u'Project',
+ required=True, vocabulary='Product'),
+ ]
+ for field in fields:
+ setUpWidget(
+ self, field.__name__, field, IInputWidget, prefix=self.name)
+ self._widgets_set_up = True
+
+ def setUpOptions(self):
+ """Set up options to be rendered."""
+ self.options = {}
+ for option in ['personal', 'product']:
+ attributes = dict(
+ type='radio', name=self.name, value=option,
+ id='%s.option.%s' % (self.name, option))
+ if self.request.form_ng.getOne(
+ self.name, self.default_option) == option:
+ attributes['checked'] = 'checked'
+ self.options[option] = renderElement('input', **attributes)
+ self.product_widget.onKeyPress = (
+ "selectWidget('%s.option.product', event)" % self.name)
+
+ def hasInput(self):
+ return self.name in self.request.form
+
+ def hasValidInput(self):
+ """See zope.app.form.interfaces.IInputWidget."""
+ try:
+ self.getInputValue()
+ return True
+ except (InputErrors, UnexpectedFormData):
+ return False
+
+ def getInputValue(self):
+ """See zope.app.form.interfaces.IInputWidget."""
+ self.setUpSubWidgets()
+ form_value = self.request.form_ng.getOne(self.name)
+ if form_value == 'product':
+ try:
+ return self.product_widget.getInputValue()
+ except MissingInputError:
+ raise LaunchpadValidationError('Please enter a project name')
+ except ConversionError:
+ entered_name = self.request.form_ng.getOne(
+ "%s.product" % self.name)
+ raise LaunchpadValidationError(
+ "There is no project named '%s' registered in"
+ " Launchpad" % entered_name)
+ elif form_value == 'personal':
+ return '+junk'
+ else:
+ raise UnexpectedFormData("No valid option was selected.")
+
+ def setRenderedValue(self, value):
+ """See IWidget."""
+ self.setUpSubWidgets()
+ if IBranchTarget.providedBy(value):
+ if IProduct.providedBy(value.context):
+ self.default_option = 'product'
+ self.product_widget.setRenderedValue(value.context)
+ return
+ elif IPerson.providedBy(value.context):
+ self.default_option = 'personal'
+ return
+ else:
+ raise AssertionError('Not a valid value: %r' % value)
+
+ def error(self):
+ """See zope.app.form.interfaces.IBrowserWidget."""
+ try:
+ if self.hasInput():
+ self.getInputValue()
+ except InputErrors as error:
+ self._error = error
+ return super(BranchTargetWidget, self).error()
+
+ def __call__(self):
+ """See zope.app.form.interfaces.IBrowserWidget."""
+ self.setUpSubWidgets()
+ self.setUpOptions()
+ return self.template()
=== added directory 'lib/lp/code/browser/widgets/templates'
=== added file 'lib/lp/code/browser/widgets/templates/branch-target.pt'
--- lib/lp/code/browser/widgets/templates/branch-target.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/widgets/templates/branch-target.pt 2012-09-14 04:09:21 +0000
@@ -0,0 +1,24 @@
+<table>
+ <tr>
+ <td colspan="2">
+ <label>
+ <input
+ type="radio" value="package"
+ tal:replace="structure view/options/personal" />
+ Personal
+ </label>
+ </td>
+ </tr>
+ <tr>
+ <td>
+ <label>
+ <input type="radio" value="product"
+ tal:replace="structure view/options/product" />
+ Project
+ </label>
+ </td>
+ <td>
+ <tal:product tal:replace="structure view/product_widget" />
+ </td>
+ </tr>
+</table>
=== added file 'lib/lp/code/browser/widgets/tests/test_branchtargetwidget.py'
--- lib/lp/code/browser/widgets/tests/test_branchtargetwidget.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/widgets/tests/test_branchtargetwidget.py 2012-09-14 04:09:21 +0000
@@ -0,0 +1,245 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import re
+
+from BeautifulSoup import BeautifulSoup
+from lazr.restful.fields import Reference
+from zope.app.form.browser.interfaces import IBrowserWidget
+from zope.app.form.interfaces import IInputWidget
+from zope.interface import (
+ implements,
+ Interface,
+ )
+
+from lp.app.validators import LaunchpadValidationError
+from lp.code.browser.widgets.branchtarget import BranchTargetWidget
+from lp.code.model.branchtarget import (
+ PersonBranchTarget,
+ ProductBranchTarget,
+ )
+from lp.registry.vocabularies import (
+ ProductVocabulary,
+ )
+from lp.services.webapp.servers import LaunchpadTestRequest
+from lp.services.webapp.testing import verifyObject
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class IThing(Interface):
+ target = Reference(schema=Interface)
+
+
+class Thing:
+ implements(IThing)
+ target = None
+
+
+class LaunchpadTargetWidgetTestCase(TestCaseWithFactory):
+ """Test the BranchTargetWidget class."""
+
+ layer = DatabaseFunctionalLayer
+
+ @property
+ def form(self):
+ return {
+ 'field.target': 'product',
+ 'field.target.product': 'pting',
+ }
+
+ def setUp(self):
+ super(LaunchpadTargetWidgetTestCase, self).setUp()
+ self.product = self.factory.makeProduct('pting')
+ field = Reference(
+ __name__='target', schema=Interface, title=u'target')
+ field = field.bind(Thing())
+ request = LaunchpadTestRequest()
+ self.widget = BranchTargetWidget(field, request)
+
+ def test_implements(self):
+ self.assertTrue(verifyObject(IBrowserWidget, self.widget))
+ self.assertTrue(verifyObject(IInputWidget, self.widget))
+
+ def test_template(self):
+ # The render template is setup.
+ self.assertTrue(
+ self.widget.template.filename.endswith('branch-target.pt'),
+ 'Template was not setup.')
+
+ def test_default_option(self):
+ # This product field is the default option.
+ self.assertEqual('product', self.widget.default_option)
+
+ def test_hasInput_false(self):
+ # hasInput is false when the widget's name is not in the form data.
+ self.widget.request = LaunchpadTestRequest(form={})
+ self.assertEqual('field.target', self.widget.name)
+ self.assertFalse(self.widget.hasInput())
+
+ def test_hasInput_true(self):
+ # hasInput is true is the widget's name in the form data.
+ self.widget.request = LaunchpadTestRequest(form=self.form)
+ self.assertEqual('field.target', self.widget.name)
+ self.assertTrue(self.widget.hasInput())
+
+ def test_setUpSubWidgets_first_call(self):
+ # The subwidgets are setup and a flag is set.
+ self.widget.setUpSubWidgets()
+ self.assertTrue(self.widget._widgets_set_up)
+ self.assertIsInstance(
+ self.widget.product_widget.context.vocabulary,
+ ProductVocabulary)
+
+ def test_setUpSubWidgets_second_call(self):
+ # The setUpSubWidgets method exits early if a flag is set to
+ # indicate that the widgets were setup.
+ self.widget._widgets_set_up = True
+ self.widget.setUpSubWidgets()
+ self.assertIs(None, getattr(self.widget, 'product_widget', None))
+
+ def test_setUpOptions_default_product_checked(self):
+ # The radio button options are composed of the setup widgets with
+ # the product widget set as the default.
+ self.widget.setUpSubWidgets()
+ self.widget.setUpOptions()
+ self.assertEqual(
+ "selectWidget('field.target.option.product', event)",
+ self.widget.product_widget.onKeyPress)
+ self.assertEqual(
+ '<input class="radioType" checked="checked" '
+ 'id="field.target.option.product" name="field.target" '
+ 'type="radio" value="product" />',
+ self.widget.options['product'])
+ self.assertEqual(
+ '<input class="radioType" '
+ 'id="field.target.option.personal" name="field.target" '
+ 'type="radio" value="personal" />',
+ self.widget.options['personal'])
+
+ def test_setUpOptions_personal_checked(self):
+ # The personal radio button is selected when the form is submitted
+ # when the target field's value is 'personal'.
+ form = {
+ 'field.target': 'personal',
+ }
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.widget.setUpSubWidgets()
+ self.widget.setUpOptions()
+ self.assertEqual(
+ '<input class="radioType" checked="checked" '
+ 'id="field.target.option.personal" name="field.target" '
+ 'type="radio" value="personal" />',
+ self.widget.options['personal'])
+ self.assertEqual(
+ '<input class="radioType" '
+ 'id="field.target.option.product" name="field.target" '
+ 'type="radio" value="product" />',
+ self.widget.options['product'])
+
+ def test_setUpOptions_product_checked(self):
+ # The product radio button is selected when the form is submitted
+ # when the target field's value is 'product'.
+ form = {
+ 'field.target': 'product',
+ }
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.widget.setUpSubWidgets()
+ self.widget.setUpOptions()
+ self.assertEqual(
+ '<input class="radioType" '
+ 'id="field.target.option.personal" name="field.target" '
+ 'type="radio" value="personal" />',
+ self.widget.options['personal'])
+ self.assertEqual(
+ '<input class="radioType" checked="checked" '
+ 'id="field.target.option.product" name="field.target" '
+ 'type="radio" value="product" />',
+ self.widget.options['product'])
+
+ def test_hasValidInput_true(self):
+ # The field input is valid when all submitted parts are valid.
+ self.widget.request = LaunchpadTestRequest(form=self.form)
+ self.assertTrue(self.widget.hasValidInput())
+
+ def test_hasValidInput_false(self):
+ # The field input is invalid if any of the submitted parts are invalid.
+ form = self.form
+ form['field.target.product'] = 'non-existent'
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.assertFalse(self.widget.hasValidInput())
+
+ def test_getInputValue_personal(self):
+ # The field value is the '+junk' when the personal radio button is
+ # selected.
+ form = self.form
+ form['field.target'] = 'personal'
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.assertEqual('+junk', self.widget.getInputValue())
+
+ def test_getInputValue_product(self):
+ # The field value is the product when the project radio button is
+ # selected and the project sub field is valid.
+ form = self.form
+ form['field.target'] = 'product'
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.assertEqual(self.product, self.widget.getInputValue())
+
+ def test_getInputValue_product_missing(self):
+ # An error is raised when the product field is missing.
+ form = self.form
+ form['field.target'] = 'product'
+ del form['field.target.product']
+ self.widget.request = LaunchpadTestRequest(form=form)
+ message = 'Please enter a project name'
+ self.assertRaisesWithContent(
+ LaunchpadValidationError, message, self.widget.getInputValue)
+ self.assertEqual(message, self.widget.error())
+
+ def test_getInputValue_product_invalid(self):
+ # An error is raised when the product is not valid.
+ form = self.form
+ form['field.target'] = 'product'
+ form['field.target.product'] = 'non-existent'
+ self.widget.request = LaunchpadTestRequest(form=form)
+ message = (
+ "There is no project named 'non-existent' registered in "
+ "Launchpad")
+ self.assertRaisesWithContent(
+ LaunchpadValidationError, message, self.widget.getInputValue)
+ self.assertEqual(message, self.widget.error())
+
+ def test_setRenderedValue_product(self):
+ # Passing a product branch target will set the widget's render state to
+ # 'product'.
+ self.widget.setUpSubWidgets()
+ target = ProductBranchTarget(self.product)
+ self.widget.setRenderedValue(target)
+ self.assertEqual('product', self.widget.default_option)
+ self.assertEqual(
+ self.product, self.widget.product_widget._getCurrentValue())
+
+ def test_setRenderedValue_personal(self):
+ # Passing a person branch target will set the widget's render state to
+ # 'personal'.
+ self.widget.setUpSubWidgets()
+ target = PersonBranchTarget(self.factory.makePerson())
+ self.widget.setRenderedValue(target)
+ self.assertEqual('personal', self.widget.default_option)
+
+ def test_call(self):
+ # The __call__ method setups the widgets and the options.
+ markup = self.widget()
+ self.assertIsNot(None, self.widget.product_widget)
+ self.assertTrue('personal' in self.widget.options)
+ expected_ids = [
+ 'field.target.option.personal',
+ 'field.target.option.product',
+ 'field.target.product',
+ ]
+ soup = BeautifulSoup(markup)
+ fields = soup.findAll(['input', 'select'], {'id': re.compile('.*')})
+ ids = [field['id'] for field in fields]
+ self.assertContentEqual(expected_ids, ids)
Follow ups
