launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:bug-1818755 into launchpad:master.

Commit message:
Added a functional Distro Series Radio buttons widget, changes to the Snap properties and a working "Create New Snap" flow.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1818755 in Launchpad itself: "The "Store Series" field of a core18 snap listed as "Ubuntu Core 16""
  https://bugs.launchpad.net/launchpad/+bug/1818755

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/375224

This is not ready for Merge, created the MP for Colin and I to look at during the 1:1 tomorrow if we have time. Current state for the MP is:

- Working UI displaying the list of Radio Buttons with all Distro Series in Postgres local instance for the Create new Snap screen (+new-snap) 
- Working UI+DB persistence of the new Snap in local Postgres when saving the Snap
- Working UI for snaps listing (+snaps) tested with 2 snaps in local Postgres added through new UI
- Many Integration Tests are currently broken on the branch - only tests with local instance of LP & PgAdmin have been performed.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:bug-1818755 into launchpad:master.

diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index 2461b29..227a785 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -97,6 +97,7 @@ from lp.snappy.interfaces.snap import (
     )
 from lp.snappy.interfaces.snapbuild import ISnapBuildSet
 from lp.snappy.interfaces.snappyseries import (
+    IDistroSeriesSet,
     ISnappyDistroSeriesSet,
     ISnappySeriesSet,
     )
@@ -344,7 +345,7 @@ class ISnapEditSchema(Interface):
         'store_upload',
         ])
     store_distro_series = Choice(
-        vocabulary='BuildableSnappyDistroSeries', required=True,
+        vocabulary='DistroSeriesVocabulary', required=True,
         title='Series')
     vcs = Choice(vocabulary=VCSType, required=True, title='VCS')
 
@@ -463,9 +464,9 @@ class SnapAddView(
             distro_series = store_series.preferred_distro_series
         else:
             distro_series = store_series.usable_distro_series.first()
-        sds_set = getUtility(ISnappyDistroSeriesSet)
-        store_distro_series = sds_set.getByBothSeries(
-            store_series, distro_series)
+        sds_set = getUtility(IDistroSeriesSet)
+        store_distro_series = sds_set.getDistroSeries(
+            distro_series.display_name)
 
         return {
             'store_name': store_name,
@@ -519,7 +520,7 @@ class SnapAddView(
             data['auto_build_pocket'] = None
         snap = getUtility(ISnapSet).new(
             self.user, data['owner'],
-            data['store_distro_series'].distro_series, data['name'],
+            data['store_distro_series'], data['name'],
             auto_build=data['auto_build'],
             auto_build_archive=data['auto_build_archive'],
             auto_build_pocket=data['auto_build_pocket'],
@@ -527,7 +528,7 @@ class SnapAddView(
             processors=data['processors'], private=private,
             build_source_tarball=data['build_source_tarball'],
             store_upload=data['store_upload'],
-            store_series=data['store_distro_series'].snappy_series,
+#            store_series=data['store_distro_series'].snappy_series,
             store_name=data['store_name'],
             store_channels=data.get('store_channels'), **kwargs)
         if data['store_upload']:
diff --git a/lib/lp/snappy/configure.zcml b/lib/lp/snappy/configure.zcml
index d062ee1..8fad099 100644
--- a/lib/lp/snappy/configure.zcml
+++ b/lib/lp/snappy/configure.zcml
@@ -146,6 +146,14 @@
             interface="lp.snappy.interfaces.snappyseries.ISnappyDistroSeriesSet" />
     </securedutility>
 
+    <!-- DistroSeriesSet -->
+    <securedutility
+        class="lp.snappy.model.snappyseries.DistroSeriesSet"
+        provides="lp.snappy.interfaces.snappyseries.IDistroSeriesSet">
+        <allow
+            interface="lp.snappy.interfaces.snappyseries.IDistroSeriesSet" />
+    </securedutility>
+
     <!-- Bases for snaps -->
     <class class="lp.snappy.model.snapbase.SnapBase">
         <allow
diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
index 70626c9..05ee32c 100644
--- a/lib/lp/snappy/interfaces/snap.py
+++ b/lib/lp/snappy/interfaces/snap.py
@@ -784,7 +784,7 @@ class ISnapEditableAttributes(IHasOwner):
 
     store_distro_series = ReferenceChoice(
         title=_("Store and distro series"),
-        schema=ISnappyDistroSeries, vocabulary="SnappyDistroSeries",
+        schema=IDistroSeries, vocabulary="DistroSeries",
         required=False, readonly=False)
 
     store_name = exported(TextLine(
diff --git a/lib/lp/snappy/interfaces/snappyseries.py b/lib/lp/snappy/interfaces/snappyseries.py
index 088806c..fa863e9 100644
--- a/lib/lp/snappy/interfaces/snappyseries.py
+++ b/lib/lp/snappy/interfaces/snappyseries.py
@@ -7,6 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'IDistroSeriesSet',
     'ISnappyDistroSeries',
     'ISnappyDistroSeriesSet',
     'ISnappySeries',
@@ -197,3 +198,13 @@ class ISnappyDistroSeriesSet(Interface):
 
     def getAll():
         """Return all `SnappyDistroSeries`."""
+
+
+class IDistroSeriesSet(Interface):
+    """Interface representing the set of distro series links."""
+
+    def getDistroSeries(distro_series):
+        """Return a `DistroSeries`, or None."""
+
+    def getAll():
+        """Return all `DistroSeries`."""
\ No newline at end of file
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index 18eee6e..4783426 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -170,7 +170,7 @@ from lp.snappy.interfaces.snapbase import (
     )
 from lp.snappy.interfaces.snapbuild import ISnapBuildSet
 from lp.snappy.interfaces.snapjob import ISnapRequestBuildsJobSource
-from lp.snappy.interfaces.snappyseries import ISnappyDistroSeriesSet
+from lp.snappy.interfaces.snappyseries import IDistroSeriesSet
 from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
 from lp.snappy.model.snapbuild import SnapBuild
 from lp.snappy.model.snapjob import SnapJob
@@ -543,13 +543,11 @@ class Snap(Storm, WebhookTargetMixin):
     def store_distro_series(self):
         if self.store_series is None:
             return None
-        return getUtility(ISnappyDistroSeriesSet).getByBothSeries(
-            self.store_series, self.distro_series)
+        return getUtility(IDistroSeriesSet).getDistroSeries(self.distro_series)
 
     @store_distro_series.setter
     def store_distro_series(self, value):
         self.distro_series = value.distro_series
-        self.store_series = value.snappy_series
 
     @property
     def store_channels(self):
diff --git a/lib/lp/snappy/model/snappyseries.py b/lib/lp/snappy/model/snappyseries.py
index 2fbbcaf..65f22b0 100644
--- a/lib/lp/snappy/model/snappyseries.py
+++ b/lib/lp/snappy/model/snappyseries.py
@@ -37,6 +37,7 @@ from lp.services.propertycache import (
     get_property_cache,
     )
 from lp.snappy.interfaces.snappyseries import (
+    IDistroSeriesSet,
     ISnappyDistroSeries,
     ISnappyDistroSeriesSet,
     ISnappySeries,
@@ -246,3 +247,20 @@ class SnappyDistroSeriesSet:
     def getAll(self):
         """See `ISnappyDistroSeriesSet`."""
         return IStore(SnappyDistroSeries).find(SnappyDistroSeries)
+
+@implementer(IDistroSeriesSet)
+class DistroSeriesSet:
+    """See `IDistroSeriesSet`."""
+
+    def getDistroSeries(self, distro_series):
+        """See `IDistroSeriesSet`."""
+        distro = IStore(DistroSeries).find(DistroSeries,
+             DistroSeries.display_name == distro_series).one()
+
+        # distro = DistroSeries.select(DistroSeries.q.display_name == distro_series)
+
+        return distro
+
+    def getAll(self):
+        """See `IDistroSeriesSet`."""
+        return IStore(DistroSeries).find(DistroSeries)
\ No newline at end of file
diff --git a/lib/lp/snappy/vocabularies.py b/lib/lp/snappy/vocabularies.py
index 331088a..3be5781 100644
--- a/lib/lp/snappy/vocabularies.py
+++ b/lib/lp/snappy/vocabularies.py
@@ -9,6 +9,7 @@ __metaclass__ = type
 
 __all__ = [
     'BuildableSnappyDistroSeriesVocabulary',
+    'DistroSeriesVocabulary',
     'SnapDistroArchSeriesVocabulary',
     'SnappyDistroSeriesVocabulary',
     'SnappySeriesVocabulary',
@@ -72,6 +73,13 @@ class SnappySeriesVocabulary(StormVocabularyBase):
     _order_by = Desc(SnappySeries.date_created)
 
 
+class DistroSeriesVocabulary(StormVocabularyBase):
+    """A vocabulary for searching distro series."""
+
+    _table = DistroSeries
+    _order_by = Desc(DistroSeries.date_created)
+
+
 class SnappyDistroSeriesVocabulary(StormVocabularyBase):
     """A vocabulary for searching snappy/distro series combinations."""
 
diff --git a/lib/lp/snappy/vocabularies.zcml b/lib/lp/snappy/vocabularies.zcml
index 8f34394..f114aa1 100644
--- a/lib/lp/snappy/vocabularies.zcml
+++ b/lib/lp/snappy/vocabularies.zcml
@@ -48,6 +48,17 @@
         <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary" />
     </class>
 
+    <securedutility
+        name="DistroSeriesVocabulary"
+        component="lp.snappy.vocabularies.DistroSeriesVocabulary"
+        provides="zope.schema.interfaces.IVocabularyFactory">
+        <allow interface="zope.schema.interfaces.IVocabularyFactory" />
+    </securedutility>
+
+    <class class="lp.snappy.vocabularies.DistroSeriesVocabulary">
+        <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary" />
+    </class>
+
     <class class="lp.snappy.vocabularies.SnapStoreChannel">
         <allow interface="zope.schema.interfaces.ITitledTokenizedTerm
                           lazr.restful.interfaces.IJSONPublishable" />