launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-auto-build-archive-widget into lp:launchpad with lp:~cjwatson/launchpad/snap-auto-build-code as a prerequisite.

Commit message:
Adjust SnapArchiveWidget to cope with its context being a branch rather than a Snap.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1593359 in Launchpad itself: "Add option to trigger snap builds when top-level branch changes"
  https://bugs.launchpad.net/launchpad/+bug/1593359

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-auto-build-archive-widget/+merge/297960

Adjust SnapArchiveWidget to cope with its context being a branch rather than a Snap.  We'll need this shortly in order to be able to include auto_build_archive in the {Branch,GitRef}:+new-snap forms.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-auto-build-archive-widget into lp:launchpad.

=== modified file 'lib/lp/snappy/browser/widgets/snaparchive.py'
--- lib/lp/snappy/browser/widgets/snaparchive.py	2015-09-21 09:00:38 +0000
+++ lib/lp/snappy/browser/widgets/snaparchive.py	2016-06-20 20:46:52 +0000
@@ -8,6 +8,7 @@
     ]
 
 from z3c.ptcompat import ViewPageTemplateFile
+from zope.component import getUtility
 from zope.formlib.interfaces import (
     ConversionError,
     IInputWidget,
@@ -25,11 +26,13 @@
 from zope.schema import Choice
 
 from lp.app.errors import UnexpectedFormData
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.validators import LaunchpadValidationError
 from lp.services.webapp.interfaces import (
     IAlwaysSubmittedWidget,
     IMultiLineWidgetLayout,
     )
+from lp.snappy.interfaces.snap import ISnap
 from lp.soyuz.interfaces.archive import IArchive
 
 
@@ -59,21 +62,28 @@
             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:
+            if (self.default_option is not None and
+                self.request.form_ng.getOne(
+                    self.name, self.default_option) == option):
                 attributes["checked"] = "checked"
             self.options[option] = renderElement("input", **attributes)
 
     @property
     def main_archive(self):
-        return self.context.context.distro_series.main_archive
+        if ISnap.providedBy(self.context.context):
+            return self.context.context.distro_series.main_archive
+        else:
+            return getUtility(ILaunchpadCelebrities).ubuntu.main_archive
 
     def setRenderedValue(self, value):
         """See `IWidget`."""
         self.setUpSubWidgets()
-        if value is None or not IArchive.providedBy(value):
+        if value is None:
+            self.default_option = None
+            self.ppa_widget.setRenderedValue(None)
+        elif not IArchive.providedBy(value):
             raise AssertionError("Not a valid value: %r" % value)
-        if value.is_primary:
+        elif value.is_primary:
             self.default_option = "primary"
             self.ppa_widget.setRenderedValue(None)
         elif value.is_ppa:
@@ -98,7 +108,9 @@
         """See `IInputWidget`."""
         self.setUpSubWidgets()
         form_value = self.request.form_ng.getOne(self.name)
-        if form_value == "primary":
+        if form_value is None:
+            return None
+        elif form_value == "primary":
             return self.main_archive
         elif form_value == "ppa":
             try:

=== modified file 'lib/lp/snappy/browser/widgets/tests/test_snaparchivewidget.py'
--- lib/lp/snappy/browser/widgets/tests/test_snaparchivewidget.py	2015-09-21 09:00:38 +0000
+++ lib/lp/snappy/browser/widgets/tests/test_snaparchivewidget.py	2016-06-20 20:46:52 +0000
@@ -7,18 +7,23 @@
 
 from BeautifulSoup import BeautifulSoup
 from lazr.restful.fields import Reference
+from zope.component import getUtility
 from zope.formlib.interfaces import (
     IBrowserWidget,
     IInputWidget,
     WidgetInputError,
     )
 
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.validators import LaunchpadValidationError
 from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.escaping import html_escape
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.snappy.browser.widgets.snaparchive import SnapArchiveWidget
-from lp.snappy.interfaces.snap import SNAP_FEATURE_FLAG
+from lp.snappy.interfaces.snap import (
+    ISnap,
+    SNAP_FEATURE_FLAG,
+    )
 from lp.soyuz.enums import ArchivePurpose
 from lp.soyuz.interfaces.archive import IArchive
 from lp.soyuz.vocabularies import PPAVocabulary
@@ -29,16 +34,17 @@
 from lp.testing.layers import DatabaseFunctionalLayer
 
 
-class TestSnapArchiveWidget(TestCaseWithFactory):
+class TestSnapArchiveWidgetMixin:
 
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(TestSnapArchiveWidget, self).setUp()
+        super(TestSnapArchiveWidgetMixin, self).setUp()
         self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+        self.distroseries = self.factory.makeDistroSeries()
         field = Reference(
             __name__="archive", schema=IArchive, title=u"Archive")
-        self.context = self.factory.makeSnap()
+        self.context = self.makeContext(self.distroseries)
         field = field.bind(self.context)
         request = LaunchpadTestRequest()
         self.widget = SnapArchiveWidget(field, request)
@@ -130,7 +136,7 @@
         # Passing a primary archive will set the widget's render state to
         # 'primary'.
         self.widget.setUpSubWidgets()
-        self.widget.setRenderedValue(self.context.distro_series.main_archive)
+        self.widget.setRenderedValue(self.distroseries.main_archive)
         self.assertEqual("primary", self.widget.default_option)
         self.assertIsNone(self.widget.ppa_widget._getCurrentValue())
 
@@ -139,10 +145,10 @@
         # 'primary', even if it was initially something else.
         self.widget.setUpSubWidgets()
         archive = self.factory.makeArchive(
-            distribution=self.context.distro_series.distribution,
+            distribution=self.distroseries.distribution,
             purpose=ArchivePurpose.PPA)
         self.widget.setRenderedValue(archive)
-        self.widget.setRenderedValue(self.context.distro_series.main_archive)
+        self.widget.setRenderedValue(self.distroseries.main_archive)
         self.assertEqual("primary", self.widget.default_option)
         self.assertIsNone(self.widget.ppa_widget._getCurrentValue())
 
@@ -150,7 +156,7 @@
         # Passing a person will set the widget's render state to 'personal'.
         self.widget.setUpSubWidgets()
         archive = self.factory.makeArchive(
-            distribution=self.context.distro_series.distribution,
+            distribution=self.distroseries.distribution,
             purpose=ArchivePurpose.PPA)
         self.widget.setRenderedValue(archive)
         self.assertEqual("ppa", self.widget.default_option)
@@ -194,13 +200,17 @@
         self.assertEqual(html_escape(message), self.widget.error())
 
     def test_getInputValue_primary(self):
-        # The field value is the context's primary archive when the primary
-        # radio button is selected.
+        # When the primary radio button is selected, the field value is the
+        # context's primary archive if the context is a Snap, or the Ubuntu
+        # primary archive otherwise.
         self.widget.request = LaunchpadTestRequest(
             form={"field.archive": "primary"})
-        self.assertEqual(
-            self.context.distro_series.main_archive,
-            self.widget.getInputValue())
+        if ISnap.providedBy(self.context):
+            expected_main_archive = self.distroseries.main_archive
+        else:
+            expected_main_archive = (
+                getUtility(ILaunchpadCelebrities).ubuntu.main_archive)
+        self.assertEqual(expected_main_archive, self.widget.getInputValue())
 
     def test_getInputValue_ppa_missing(self):
         # An error is raised when the ppa field is missing.
@@ -243,3 +253,24 @@
             ]
         ids = [field["id"] for field in fields]
         self.assertContentEqual(expected_ids, ids)
+
+
+class TestSnapArchiveWidgetForSnap(
+    TestSnapArchiveWidgetMixin, TestCaseWithFactory):
+
+    def makeContext(self, distroseries):
+        return self.factory.makeSnap(distroseries=distroseries)
+
+
+class TestSnapArchiveWidgetForBranch(
+    TestSnapArchiveWidgetMixin, TestCaseWithFactory):
+
+    def makeContext(self, distroseries):
+        return self.factory.makeAnyBranch()
+
+
+class TestSnapArchiveWidgetForGitRepository(
+    TestSnapArchiveWidgetMixin, TestCaseWithFactory):
+
+    def makeContext(self, distroseries):
+        return self.factory.makeGitRepository()