launchpad-reviewers team mailing list archive

[Bryce Harrington <bryce@xxxxxxxxxxxxxxxxxxx>]

You have been requested to review the proposed merge of lp:~bryce/launchpad/lp-617695-linkui into lp:launchpad.

This branch contains the UI changes to implement display of components and component groups (products) available from a given bug tracker, along with an edit form for linking components to particular source packages.

A preimplementation call was done with Deryck about the design and implementation of the ui and code.  He also gave some advice on the tests while we were at UDS.

Lint issues have been checked and fixed.

I test this using the command:

  ./bin/test -t browser.*bugtracker_component


-- 
https://code.launchpad.net/~bryce/launchpad/lp-617695-linkui/+merge/39679
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bryce/launchpad/lp-617695-linkui into lp:launchpad.

=== modified file 'lib/canonical/launchpad/scripts/bzremotecomponentfinder.py'
--- lib/canonical/launchpad/scripts/bzremotecomponentfinder.py	2010-10-20 00:24:50 +0000
+++ lib/canonical/launchpad/scripts/bzremotecomponentfinder.py	2010-11-02 06:22:44 +0000
@@ -117,7 +117,6 @@
         self.static_bugzilla_text = static_bugzilla_text
 
     def getRemoteProductsAndComponents(self, bugtracker_name=None):
-        """"""
         lp_bugtrackers = getUtility(IBugTrackerSet)
         if bugtracker_name is not None:
             lp_bugtrackers = [

=== modified file 'lib/canonical/widgets/bugtask.py'
--- lib/canonical/widgets/bugtask.py	2010-10-03 15:30:06 +0000
+++ lib/canonical/widgets/bugtask.py	2010-11-02 06:22:44 +0000
@@ -491,6 +491,25 @@
         return distribution
 
 
+class UbuntuSourcePackageNameWidget(
+    BugTaskSourcePackageNameWidget):
+    """Package widget where the distribution can be assumed as Ubuntu
+
+    This widgets works the same as `BugTaskSourcePackageNameWidget`,
+    except that it assumes the distribution is 'ubuntu'.
+    """
+    distribution_name = "ubuntu"
+
+    def getDistribution(self):
+        """See `BugTaskSourcePackageNameWidget`"""
+        distribution = getUtility(IDistributionSet).getByName(
+            self.distribution_name)
+        if distribution is None:
+            raise UnexpectedFormData(
+                "No such distribution: %s" % self.distribution_name)
+        return distribution
+
+
 class AssigneeDisplayWidget(BrowserWidget):
     """A widget for displaying an assignee."""
 

=== modified file 'lib/lp/bugs/browser/bugtracker.py'
--- lib/lp/bugs/browser/bugtracker.py	2010-10-15 08:23:19 +0000
+++ lib/lp/bugs/browser/bugtracker.py	2010-11-02 06:22:44 +0000
@@ -10,6 +10,7 @@
     'BugTrackerBreadcrumb',
     'BugTrackerComponentGroupNavigation',
     'BugTrackerEditView',
+    'BugTrackerEditComponentView',
     'BugTrackerNavigation',
     'BugTrackerNavigationMenu',
     'BugTrackerSetBreadcrumb',
@@ -21,6 +22,7 @@
     ]
 
 from itertools import chain
+from storm.locals import Store
 
 from zope.app.form.browser import TextAreaWidget
 from zope.component import getUtility
@@ -67,13 +69,19 @@
     DelimitedListWidget,
     LaunchpadRadioWidget,
     )
+from canonical.widgets.bugtask import (
+    UbuntuSourcePackageNameWidget,
+    )
 from lp.bugs.interfaces.bugtracker import (
     BugTrackerType,
     IBugTracker,
     IBugTrackerSet,
     IRemoteBug,
+    IBugTrackerComponent,
     IBugTrackerComponentGroup,
     )
+from lp.bugs.model.bugtracker import BugTrackerComponent
+from lp.registry.interfaces.distribution import IDistributionSet
 from lp.services.propertycache import cachedproperty
 
 # A set of bug tracker types for which there can only ever be one bug
@@ -229,6 +237,14 @@
         return shortlist(chain(self.context.projects,
                                self.context.products), 100)
 
+    @property
+    def related_component_groups(self):
+        """Return all component groups and components
+
+        This property was created for the Related components portlet in
+        the bug tracker's page.
+        """
+        return self.context.getAllRemoteComponentGroups()
 
 BUG_TRACKER_ACTIVE_VOCABULARY = SimpleVocabulary.fromItems(
     [('On', True), ('Off', False)])
@@ -447,16 +463,113 @@
             return RemoteBug(self.context, remotebug, bugs)
 
     @stepthrough("+components")
-    def component_groups(self, name):
-        return self.context.getRemoteComponentGroup(name)
+    def component_groups(self, id):
+        # Navigate by id (component group name should work too)
+        return self.context.getRemoteComponentGroup(id)
+
+
+class BugTrackerEditComponentView(LaunchpadEditFormView):
+    """Provides editing form for setting source packages for components.
+
+    In this class we assume that bug tracker components are always
+    linked to source packages in the Ubuntu distribution.
+    """
+
+    schema = IBugTrackerComponent
+    custom_widget('sourcepackagename',
+                  UbuntuSourcePackageNameWidget)
+
+    @property
+    def page_title(self):
+        return smartquote(
+            u'Link a distribution source package to the %s component'
+            % self.context.name)
+
+    @property
+    def field_names(self):
+        field_names = [
+            'sourcepackagename',
+            ]
+        return field_names
+
+    def setUpWidgets(self, context=None):
+        for field in self.form_fields:
+            if (field.custom_widget is None and
+                field.__name__ in self.custom_widgets):
+                field.custom_widget = self.custom_widgets[field.__name__]
+        self.widgets = form.setUpWidgets(
+            self.form_fields, self.prefix, self.context, self.request,
+            data=self.initial_values, adapters=self.adapters,
+            ignore_request=False)
+
+    @property
+    def initial_values(self):
+        """See `LaunchpadFormView.`"""
+        field_values = {}
+        for name in self.field_names:
+            if name == 'sourcepackagename':
+                pkg = self.context.distro_source_package
+                if pkg is not None:
+                    field_values['sourcepackagename'] = pkg.name
+                else:
+                    field_values['sourcepackagename'] = ""
+            else:
+                field_values[name] = getattr(self.context, name)
+
+        return field_values
+
+    def updateContextFromData(self, data, context=None):
+        """Link component to specified distro source package.
+
+        Get the user-provided source package name from the form widget,
+        look it up in Ubuntu to retrieve the distro_source_package
+        object, and link it to this component.
+        """
+        if context is None:
+            context = self.context
+        component = context
+
+        sourcepackagename = self.request.form.get(
+            self.widgets['sourcepackagename'].name)
+
+        distro_name = self.widgets['sourcepackagename'].distribution_name
+        distribution = getUtility(IDistributionSet).getByName(distro_name)
+        pkg = distribution.getSourcePackage(sourcepackagename)
+
+        # Has this source package already been assigned to a component?
+        pkgs = Store.of(component).find(
+            BugTrackerComponent,
+            BugTrackerComponent.distribution == distribution.id,
+            BugTrackerComponent.source_package_name ==
+            pkg.sourcepackagename.id)
+        if pkgs.count()>0:
+            self.request.response.addNotification(
+                "The %s source package is already linked to %s:%s in %s" %(
+                    sourcepackagename, component.component_group.name,
+                    component.name, distro_name))
+            return
+
+        component.distro_source_package = pkg
+        self.request.response.addNotification(
+            "%s:%s is now linked to the %s source package in %s" %(
+                component.component_group.name, component.name,
+                sourcepackagename, distro_name))
+
+    @action('Save Changes', name='save')
+    def save_action(self, action, data):
+        """Update the component with the form data."""
+        self.updateContextFromData(data)
+
+        self.next_url = canonical_url(
+            self.context.component_group.bug_tracker)
 
 
 class BugTrackerComponentGroupNavigation(Navigation):
 
     usedfor = IBugTrackerComponentGroup
 
-    def traverse(self, name):
-        return self.context.getComponent(name)
+    def traverse(self, id):
+        return self.context.getComponent(id)
 
 
 class BugTrackerSetBreadcrumb(Breadcrumb):

=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml	2010-10-28 09:11:36 +0000
+++ lib/lp/bugs/browser/configure.zcml	2010-11-02 06:22:44 +0000
@@ -811,6 +811,12 @@
             path_expression="name"
             attribute_to_parent="component_group"
             rootsite="bugs"/>
+        <browser:page
+            name="+edit"
+            for="lp.bugs.interfaces.bugtracker.IBugTrackerComponent"
+            class="lp.bugs.browser.bugtracker.BugTrackerEditComponentView"
+            permission="launchpad.AnyPerson"
+            template="../templates/bugtracker-edit-component.pt"/>
         <browser:pages
             for="lp.bugs.interfaces.bugtracker.IBugTracker"
             class="lp.bugs.browser.bugtracker.BugTrackerView"
@@ -822,6 +828,9 @@
                 name="+portlet-details"
                 template="../templates/bugtracker-portlet-details.pt"/>
             <browser:page
+                name="+portlet-components"
+                template="../templates/bugtracker-portlet-components.pt"/>
+            <browser:page
                 name="+portlet-projects"
                 template="../templates/bugtracker-portlet-projects.pt"/>
             <browser:page

=== added file 'lib/lp/bugs/browser/tests/test_bugtracker_component.py'
--- lib/lp/bugs/browser/tests/test_bugtracker_component.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugtracker_component.py	2010-11-02 06:22:44 +0000
@@ -0,0 +1,116 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version (see the file LICENSE).
+
+"""Unit tests for linking bug tracker components to source packages."""
+
+__metaclass__ = type
+
+import unittest
+from zope.component import getUtility
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.registry.interfaces.distribution import IDistributionSet
+from lp.testing import (
+    login_person,
+    TestCaseWithFactory,
+    )
+from lp.testing.views import create_initialized_view
+
+
+class TestBugTrackerEditComponentView(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestBugTrackerEditComponentView, self).setUp()
+        regular_user = self.factory.makePerson()
+        login_person(regular_user)
+
+        self.bug_tracker = self.factory.makeBugTracker()
+        self.comp_group = self.factory.makeBugTrackerComponentGroup(
+            u'alpha', self.bug_tracker)
+
+    def _makeForm(self, sourcepackage):
+        if sourcepackage is None:
+            name = ''
+        else:
+            name = sourcepackage.name
+        return {
+            'field.sourcepackagename': name,
+            'field.actions.save': 'Save',
+            }
+
+    def test_view_attributes(self):
+        component = self.factory.makeBugTrackerComponent(
+            u'Example', self.comp_group)
+        distro = getUtility(IDistributionSet).getByName('ubuntu')
+        package = self.factory.makeDistributionSourcePackage(
+            sourcepackagename='example', distribution=distro)
+        form = self._makeForm(package)
+        view = create_initialized_view(
+            component, name='+edit', form=form)
+        label = 'Link a distribution source package to the Example component'
+        self.assertEqual(label, view.page_title)
+        fields = ['sourcepackagename']
+        self.assertEqual(fields, view.field_names)
+
+    def test_linking(self):
+        component = self.factory.makeBugTrackerComponent(
+            u'Example', self.comp_group)
+        distro = getUtility(IDistributionSet).getByName('ubuntu')
+        package = self.factory.makeDistributionSourcePackage(
+            sourcepackagename='example', distribution=distro)
+
+        self.assertIs(None, component.distro_source_package)
+        form = self._makeForm(package)
+        view = create_initialized_view(
+            component, name='+edit', form=form)
+        self.assertEqual([], view.errors)
+
+        notifications = view.request.response.notifications
+        #self.assertEqual(1, len(notifications))
+        self.assertEqual(component.distro_source_package, package)
+        expected = (
+            u"alpha:Example is now linked to the example "
+            "source package in ubuntu")
+        self.assertEqual(expected, notifications.pop().message)
+
+    def test_cannot_doublelink_sourcepackages(self):
+        # Two components try linking to same package
+        component_a = self.factory.makeBugTrackerComponent(
+            u'a', self.comp_group)
+        component_b = self.factory.makeBugTrackerComponent(
+            u'b', self.comp_group)
+        distro = getUtility(IDistributionSet).getByName('ubuntu')
+        package = self.factory.makeDistributionSourcePackage(
+            sourcepackagename='example', distribution=distro)
+
+        form = self._makeForm(package)
+        view = create_initialized_view(
+            component_a, name='+edit', form=form)
+        notifications = view.request.response.notifications
+        self.assertEqual([], view.errors)
+        self.assertEqual(1, len(notifications))
+        self.assertEqual(package, component_a.distro_source_package)
+
+        form = self._makeForm(package)
+        view = create_initialized_view(
+            component_b, name='+edit', form=form)
+        self.assertIs(None, component_b.distro_source_package)
+        self.assertEqual([], view.errors)
+        notifications = view.request.response.notifications
+        self.assertEqual(1, len(notifications))
+        expected = (
+            u"""The example source package is already linked to """
+            """alpha:b in ubuntu""")
+        self.assertEqual(expected, notifications.pop().message)
+
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))
+
+    return suite
+
+if __name__ == '__main__':
+    unittest.TextTestRunner().run(test_suite())

=== modified file 'lib/lp/bugs/interfaces/bugtracker.py'
--- lib/lp/bugs/interfaces/bugtracker.py	2010-10-15 01:37:04 +0000
+++ lib/lp/bugs/interfaces/bugtracker.py	2010-11-02 06:22:44 +0000
@@ -521,6 +521,10 @@
             title=_('Name'),
             description=_("The name of a software component "
                           "as shown in Launchpad.")))
+    sourcepackagename = Choice(
+        title=_("Package"), required=False, vocabulary='SourcePackageName')
+    distribution = Choice(
+        title=_("Distribution"), required=False, vocabulary='Distribution')
 
     distro_source_package = exported(
         Reference(

=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py	2010-10-19 01:21:14 +0000
+++ lib/lp/bugs/model/bugtracker.py	2010-11-02 06:22:44 +0000
@@ -269,10 +269,17 @@
 
         if component_name is None:
             return None
+        elif component_name.isdigit():
+            component_id = int(component_name)
+            return Store.of(self).find(
+                BugTrackerComponent,
+                BugTrackerComponent.id == component_id,
+                BugTrackerComponent.component_group == self.id).one()
         else:
             return Store.of(self).find(
                 BugTrackerComponent,
-                (BugTrackerComponent.name == component_name)).one()
+                BugTrackerComponent.name == component_name,
+                BugTrackerComponent.component_group == self.id).one()
 
     def addCustomComponent(self, component_name):
         """Adds a component locally that isn't synced from a remote tracker
@@ -671,9 +678,15 @@
         """See `IBugTracker`."""
         component_group = None
         store = IStore(BugTrackerComponentGroup)
-        component_group = store.find(
-            BugTrackerComponentGroup,
-            name = component_group_name).one()
+        if component_group_name.isdigit():
+            component_group_id = int(component_group_name)
+            component_group = store.find(
+                BugTrackerComponentGroup,
+                id = component_group_id).one()
+        else:
+            component_group = store.find(
+                BugTrackerComponentGroup,
+                name = component_group_name).one()
         return component_group
 
 

=== added file 'lib/lp/bugs/templates/bugtracker-edit-component.pt'
--- lib/lp/bugs/templates/bugtracker-edit-component.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/templates/bugtracker-edit-component.pt	2010-11-02 06:22:44 +0000
@@ -0,0 +1,16 @@
+<bug-tracker-edit-component
+  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="malone">
+
+  <div metal:fill-slot="main">
+    <h1>Link <span tal:replace="context/component_group/name"/> component '<span tal:replace="context/name"/>'</h1>
+    <div metal:use-macro="context/@@launchpad_form/form">
+      <p>Configure component</p>
+    </div>
+  </div>
+
+</bug-tracker-edit-component>

=== modified file 'lib/lp/bugs/templates/bugtracker-index.pt'
--- lib/lp/bugs/templates/bugtracker-index.pt	2009-09-01 15:58:46 +0000
+++ lib/lp/bugs/templates/bugtracker-index.pt	2010-11-02 06:22:44 +0000
@@ -34,6 +34,9 @@
     <div class="yui-g">
       <div class="first yui-u">
         <div tal:replace="structure context/@@+portlet-details" />
+        <div tal:condition="features/bugtracker_components">
+          <div tal:replace="structure context/@@+portlet-components" />
+        </div>
       </div>
       <div class="yui-u">
         <div tal:replace="structure context/@@+portlet-projects" />

=== added file 'lib/lp/bugs/templates/bugtracker-portlet-components.pt'
--- lib/lp/bugs/templates/bugtracker-portlet-components.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/templates/bugtracker-portlet-components.pt	2010-11-02 06:22:44 +0000
@@ -0,0 +1,37 @@
+<div
+    xmlns:tal="http://xml.zope.org/namespaces/tal";
+    xmlns:metal="http://xml.zope.org/namespaces/metal";
+    xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+    class="portlet" id="portlet-components">
+  <h2>Components</h2>
+  <p>
+    You can link components from this bug tracker to their corresponding
+    distribution source packages in the project's &ldquo;Change
+    components&rdquo; page.
+  </p>
+  <ul tal:define="related_component_groups view/related_component_groups">
+    <li tal:repeat="component_group related_component_groups">
+      <strong><span tal:replace="component_group/name" /></strong>
+      <ul tal:define="components component_group/components">
+        <li tal:repeat="component components">
+          <span tal:replace="component/name" />
+          &nbsp;
+          <span tal:condition="component/distro_source_package">
+            <a class="sprite edit"
+               tal:attributes="href string:${context/fmt:url}/+components/${component_group/id}/${component/id}/+edit">
+               <span tal:replace="structure component/distro_source_package/name"/></a>
+          </span>
+          <a class="sprite add"
+             tal:condition="not: component/distro_source_package"
+             tal:attributes="href string:${context/fmt:url}/+components/${component_group/id}/${component/id}/+edit"></a>
+        </li>
+        <li tal:condition="not: components">
+          <i>This bug tracker has no components for this group</i>
+        </li>
+      </ul>
+    </li>
+    <li tal:condition="not: related_component_groups">
+      <i>This bug tracker has no components</i>
+    </li>
+  </ul>
+</div>