Merge ~ilasc/launchpad:bug-1818755 into launchpad:master

[Ioana Lasc]

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

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.

8add3c5... by Ioana Lasc on 2019-11-07

Added fix for Edit Snap View - was breaking for the new Distro Series Radio Widget.

ff40a3c... by Ioana Lasc on 2019-11-07

Added fix for Update Snap Package action - was breaking with the switch to the new Distro Series only Radio Widget.

a158b61... by Ioana Lasc on 2019-11-12

Editing vocabulary for Distro Series instead of creating a new one.

239c812... by Ioana Lasc on 2019-11-13

Altered Vocabulary to display the Store Series in the Edit list for upgrades from value 1 to 2.

3428c06... by Ioana Lasc on 2019-11-13

Added appropriate queries in the model to fetch the correct list of Bulidable Distro Series.

2aed2c9... by Ioana Lasc on 2019-11-14

Working code for the String Based Vocabulary approach - needs cleanup.

7ba1cbd... by Ioana Lasc on 2019-11-15

Small code cleanup.

[Ioana Lasc (ilasc)]

This is an "kindly asking for help" type of review request.

There is a modified Test Class name and other aesthetic bits (i.e. string manipulation through list indexes) that I am well aware of and can be ignored as they will definitely be addressed when switching to the new Vocabulary - backed by DB which is essentially the part I need help with.

[Colin Watson (cjwatson)]

So on reflection I think the attempt to use a non-DB-backed vocabulary for this was a serious red herring that I should have steered you away from sooner. I thought it was more of a brief temporary experiment, but it's led you down the path of making quite time-consuming changes all over the place to support it that aren't going to be mergeable. Similarly, hardcoding IDs is never going to be the right thing to do and I think relying on them has perhaps got in the way of better approaches. I hope these suggestions get you back on the right path.

Regarding how to do the presentational side of these changes, particularly the parts in SnapAddView and SnapEditView: as I mentioned before, there are a couple of possible approaches. We do IMO want to keep the approach where store_series and distro_series are only ever edited in combined form in the UI, even if the store_series part of that is mostly vestigial. That means that we need to figure out a way to represent those combinations in a way that's less confusing.

At the moment, store_distro_series is presented using a LaunchpadRadioWidget, which just renders as a list of radio buttons with choices from the vocabulary. That's fine - there doesn't seem to be a particular need to change that, so leave it alone. The vocabulary's terms are "titled": that is, as well as having a value (a SnappyDistroSeries object) and a token (a machine-readable identifier which can be used in HTML forms), they also have a title which is intended for display to humans. The title seems like the right thing to change in this case.

Now, a vocabulary can either make up its own titles, or it can delegate that to something else. If you look at SnappyDistroSeriesVocabulary.toTerm, it returns SimpleTerm(obj, token, obj.title), meaning that it's just using the "title" property of SnappyDistroSeries. That does mean we're leaking presentational details down into the model layer a bit, but that's relatively harmless in this case as IIRC that title isn't used by anything else. So we probably might as well just decide on what each of the choices should look like in the UI and then make SnappyDistroSeries.title return those.

cb62878... by Ioana Lasc on 2019-11-25

Moved to a DB backed BuildableSnappyDistroSeriesVocabulary.

ffd2234... by Ioana Lasc on 2019-12-02

Moved to materializing Vocabulary to List approach

[Ioana Lasc (ilasc)]

This definitely not ready for a full review - a lot of cleanup to be done - but before I spend time going forward with this approach I think it's wise to get a first round of feedback on the materialized list in entries, particularly with focus on the 'toTerm' method in SnappyDistroSeriesVocabulary and '_entries' in BuildableSnappyDistroSeriesVocabulary.

[Colin Watson (cjwatson)]

I know you said that there was a lot of cleanup to be done, but I'm concerned that you're getting lost in the weeds here, especially because the approach here doesn't seem to match the advice I gave when we last talked about it, and the direction of travel is going to make things very difficult to maintain.

I've tried to be a bit more specific about what I'm expecting in comments here (though I've tried to identify the main problems rather than doing a full line-by-line), but if anything is at all unclear please get in touch with me as soon as possible - no need to wait for a standup meeting or similar.

review: Needs Fixing

91c6498... by Ioana Lasc on 2019-12-03

Implemented the synthetic SnappyDistroSeries object as a type for the materialized list returned from BuildableSnappyDistroSeriesVocabulary

[Ioana Lasc (ilasc)]

Unit Tests will take a while to write & fix and before I go down that time-sync I thought I'd do what I said I would and update the MP with code that needs to be on the right path in order to spend time further on implementation.
This code works locally only for displaying a list for Create and Edit Snap (based on Current and Older Store Series).

On the topic of the 4 situations that we want to capture with the new title formatting:
1: Default Case  -> this code currently shows Ubuntu core 16
2: For Current Store Series - > this code currently shows: just the Distro Series
3: For Future Store Series versions -> this code currently shows: Distro Series for Store Series
4: For Older Store Series -> this code currently shows: Distro Series for Store Series

I'm working on trying to come up with something next that will hopefully work well for all 4 situations.

[Colin Watson (cjwatson)]

This is looking much better, thanks! Some notes follow.

58dc01a... by Ioana Lasc on 2019-12-04

Added enhancements to BuildableSnappyDistroSeriesVocabulary and a first Unit Test with required changes.

[Ioana Lasc (ilasc)]

Thanks Colin!

Addressed a few of the previous review comments and added a first Unit Test with a few changes required to be able to not create a second makeSnappySeries() in factory.py - currently breaks a few existing unit tests on the branch.

Updated the MP for early feedback on changes so far in case anything looks out of place.

Still to do:

1. move the check for (context == Snap) to SnappyDistroSeriesVocabulary
2. complete changes for title alterations per Colin's spec
3. Add Unit Tests and fix the currently broken ones.

8cbf47d... by Ioana Lasc on 2019-12-04

Moved the check for (context == Snap) to SnappyDistroSeriesVocabulary and deleted BuildableSnappyDistroSeriesVocabulary.

[Ioana Lasc (ilasc)]

Moved the check for (context == Snap) to SnappyDistroSeriesVocabulary and deleted BuildableSnappyDistroSeriesVocabulary. I didn't see the point in keeping the BuildableSnappyDistroSeriesVocabulary - might be wrong but it looked like there's nothing left in it.

Still to do:

2. complete changes for title alterations per Colin's spec
3. Add Unit Tests and fix the currently broken ones.

[Colin Watson (cjwatson)]

Hmm, I suppose it's probably OK (and indeed less confusing) to delete the Buildable version now that the basic version is more sophisticated, but in that case could you please add the `SnappySeries.status.is_in(ACTIVE_STATUSES)` clause to SnappyDistroSeriesVocabulary? It doesn't make a difference on production at the moment, but in principle it might in the future.

Also if you could clean up import ordering and such (utilities/format-new-and-modified-imports) that would make things easier to read.

Otherwise, getting there, so I've done a bit more of a line-by-line review.

0d0f607... by Ioana Lasc on 2019-12-09

Adjusted title structure for Snappyseries and moved from a DB OderBy clause to a python list sorting.

7f45f58... by Ioana Lasc on 2019-12-09

Removed spurious tab from SnappyDistroSeriesVocabulary.toTerm.

[Ioana Lasc (ilasc)]

MP is now ready for review. Thanks Colin!

[Colin Watson (cjwatson)]

I suggested earlier that some unit tests for SnappyDistroSeriesVocabulary might be a good idea. I now think that's definitely needed: there are enough special cases in here that it doesn't make sense to try to test for them only in passing in browser-level tests.

Aside from that, I think we're now converging on something that will work, but I've left a number of detailed comments. Thanks.

a453a3d... by Ioana Lasc on 2020-01-10

Addressed code review comments and added Unit Tests.

[Ioana Lasc (ilasc)]

Thanks Colin for the comments and suggestions on the last round, hopefully we're now closer now.

It's ready now for another review.

Worked through this list of direct issues:

1 Vocabulary sorting function was crashing on entries where distro_series was None.
2 The sort function used distro_series.display_name rather than distro_series.distribution.display_name
3 Sorting entries in Vocabulary in descending order by date_created - using a convertion to epoch
4 Existing Unit Tests were failing for titles with Store Series None - it was a problem with the Vocabulary code.
5 The bullet list didn't meet the mini spec - was missing the "Infer from snapcraft.yaml (recommended)" entry
6 Title needs to be the same for SnappyDistroSeries and SyntheticSnappyDistroSeries
7 More Unit Tests in Browser
8 Add Vocabulary Unit Tests

[Colin Watson (cjwatson)]

Thanks for working through all this! This is looking much better now. I've done a line-by-line review with some specific issues, mostly style but with a couple of functional issues; I think you can land this once those are fixed.

review: Approve

[Colin Watson (cjwatson)]

Also, could you update the commit message and description on this MP, which I think date from much earlier versions? The commit message should ideally be more along the lines of https://commit.style/ too.

[Colin Watson (cjwatson)]

I just noticed that I left an Approve vote claiming that I'd done a line-by-line review, but there's no record of that review to be found. Whoops! I've done my best to reconstruct what I probably would have said.

8971bd4... by Ioana Lasc on 2020-07-27

Merge branch 'master' into bug-1818755

5b4762b... by Ioana Lasc on 2020-07-28

Address code review comments

[Ioana Lasc (ilasc)]

Thank you Colin, MP ready for another look, all comments addressed apart from 1 as not sure why the Unit Test is not failing - I left the details inline as reply to your previous comments - line 379 in this MP.

0a68638... by Ioana Lasc on 2020-08-12

test_edit_snap_built_for_snappy_series_None fails

[Ioana Lasc (ilasc)]

The test that is failing for this MP is test_edit_snap_built_for_snappy_series_None in
lib/lp/snappy/browser/tests/test_snap.py with:

Traceback (most recent call last):
  File "/home/ilasc/launchpad/launchpad/lib/lp/snappy/browser/tests/test_snap.py", line 847, in test_edit_snap_built_for_snappy_series_None
    self.assertEqual([], find_tags_by_class(browser.contents, "message"))
  File "/home/ilasc/launchpad/launchpad/env/local/lib/python2.7/site-packages/testtools/testcase.py", line 415, in assertEqual
    self.assertThat(observed, matcher, message)
  File "/home/ilasc/launchpad/launchpad/env/local/lib/python2.7/site-packages/testtools/testcase.py", line 502, in assertThat
    raise mismatch_error
testtools.matchers._impl.MismatchError: !=:
reference = []
actual = [<p class="error message">There is 1 error.</p>,
 <div class="message">Constraint not satisfied</div>]

I have seen the same type of failure in the other tests:
test_edit_snap_built_for_distro_series_None
test_edit_snap_built_for_older_store_series

and fixed it by adding:
                 usable_distro_series=[distro_series],

when constructing the snappy_series object.

[Thiago F. Pappacena (pappacena)]

ilasc, the error you are having is caused by the validation on ISnapEditSchema.store_distro_series field.

I had to debug a bit, and I admit that it was kind of random at first, until I could narrow down. Basically, I have put an ipdb at BaseSnapEditView.validate method, saw that `data` was missing `store_distro_series` value, checked that this is a Choice field with `vocabulary='SnappyDistroSeries', required=True`.

With that, I had a quick look on how Choices validates vocabulary (zope.schema._field.Choice._validate), and it just uses a simple `if value not in vocabulary`, meaning that putting a break point at SnappyDistroSeriesVocabulary.__contains__ would probably help me to find out what was going on.

Long story short, the vocabulary's getTermByToken method returns SyntheticSnappyDistroSeries objects in this test, and __contains__ is not prepared for that.

Basically, changing __contains__ and making sure that SyntheticSnappyDistroSeries has proper accessors permissions set on zcml made the test pass:

- Code diff: https://pastebin.canonical.com/p/BfwCJ8Cp9b/
- Test output: https://pastebin.canonical.com/p/w7Rcmph3gN/

This technically solves the issue with this test, but please check if that makes sense for the whole task point of view (if the permission is correct, and we really should accept any SyntheticSnappyDistroSeries as being part of the Vocabulary.

63274c2... by Ioana Lasc on 2020-08-17

Fix test_edit_snap_built_for_snappy_series_None

[Ioana Lasc (ilasc)]

Thanks Thiago!
It makes sense to me from a task scope perspective but there might be more complex implications I might be missing in terms of permissions?
Colin this is now ready for another look.

[Colin Watson (cjwatson)]

Just assuming that any SyntheticSnappyDistroSeries is part of the vocabulary will probably bite us at some point. I suggest implementing an equality test for it instead so that __contains__ can work more naturally, perhaps as in this patch:

  https://paste.ubuntu.com/p/9kVWX5swyG/

bab382b... by Ioana Lasc on 2020-08-21

Add equality test for SyntheticSnappyDistroSeries

[Ioana Lasc (ilasc)]

Agreed on above point Colin, your patch makes sens, applied and tested locally.
MP now ready for another look.

[Colin Watson (cjwatson)]

The problem I've mentioned below in the case where both distro_series and store_series of an existing snap are None needs to be fixed before landing this, but this otherwise looks pretty close. Thanks!

review: Approve

a396d18... by Ioana Lasc on 2020-09-08

Handle both snappy and distro series being null

[Ioana Lasc (ilasc)]

Thanks Colin!
Agreed it definitely needed handling for both Snappy and Distro series being passed in as null.
MP now ready for review.

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index 643e989..bda9483 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -351,7 +351,7 @@ class ISnapEditSchema(Interface):
         'store_upload',
         ])
     store_distro_series = Choice(
-        vocabulary='BuildableSnappyDistroSeries', required=True,
+        vocabulary='SnappyDistroSeries', required=True,
         title='Series')
     vcs = Choice(vocabulary=VCSType, required=True, title='VCS')
 
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index 149d289..10e82da 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -128,7 +128,8 @@ class TestSnapNavigation(TestCaseWithFactory):
     def test_snap(self):
         snap = self.factory.makeSnap()
         obj, _, _ = test_traverse(
-            "http://launchpad.test/~%s/+snap/%s" % (snap.owner.name, snap.name))
+            "http://launchpad.test/~%s/+snap/%s" % (
+                snap.owner.name, snap.name))
         self.assertEqual(snap, obj)
 
 
@@ -778,6 +779,92 @@ class TestSnapEditView(BaseTestSnapView):
             "the store.\nEdit snap package",
             MatchesTagText(content, "store_upload"))
 
+    def test_edit_snap_built_for_older_store_series(self):
+        distro_series = self.factory.makeUbuntuDistroSeries()
+        with admin_logged_in():
+            snappy_series = self.factory.makeSnappySeries(
+                usable_distro_series=[distro_series],
+                status=SeriesStatus.SUPPORTED)
+        snap = self.factory.makeSnap(
+            registrant=self.person, owner=self.person,
+            distroseries=distro_series,
+            store_series=snappy_series,
+            branch=self.factory.makeAnyBranch())
+        browser = self.getViewBrowser(snap, view_name="+edit", user=snap.owner)
+        browser.getControl(name="field.store_distro_series").value = (
+            "ubuntu/%s/%s" % (distro_series.name, snappy_series.name))
+        browser.getControl("Update snap package").click()
+
+        self.assertEqual([], find_tags_by_class(browser.contents, "message"))
+        login_person(self.person)
+        self.assertThat(snap, MatchesStructure.byEquality(
+            distro_series=distro_series,
+            store_series=snappy_series))
+
+    def test_edit_snap_built_for_distro_series_None(self):
+        with admin_logged_in():
+            snappy_series = self.factory.makeSnappySeries(
+                status=SeriesStatus.CURRENT)
+        snap = self.factory.makeSnap(
+            registrant=self.person, owner=self.person,
+            distroseries=None,
+            store_series=snappy_series)
+        browser = self.getViewBrowser(snap, user=self.person)
+        browser.getLink("Edit snap package").click()
+        browser.getControl(
+            name="field.store_distro_series").value = (
+                        browser.getControl(
+                            name="field.store_distro_series"
+                        ).options[0].strip())
+        browser.getControl("Update snap package").click()
+        self.assertEqual([], find_tags_by_class(browser.contents, "message"))
+        login_person(self.person)
+        self.assertThat(snap, MatchesStructure(
+            distro_series=Is(None),
+            store_series=Equals(snappy_series)))
+
+    def test_edit_snap_built_for_snappy_series_None(self):
+        distro_series = self.factory.makeUbuntuDistroSeries()
+
+        snap = self.factory.makeSnap(
+            registrant=self.person, owner=self.person,
+            distroseries=distro_series,
+            store_series=None)
+
+        browser = self.getViewBrowser(snap, view_name="+edit", user=snap.owner)
+        self.assertIn(
+            "ubuntu/%s" % distro_series.name,
+            browser.getControl(name="field.store_distro_series").options)
+        browser.getControl(
+            name="field.store_distro_series").value = (
+                        "ubuntu/%s" % distro_series.name)
+        browser.getControl("Update snap package").click()
+        self.assertEqual([], find_tags_by_class(browser.contents, "message"))
+        login_person(self.person)
+        self.assertThat(snap, MatchesStructure(
+            distro_series=Equals(distro_series),
+            store_series=Is(None)))
+
+    def test_edit_snap_built_for_distro_snappy_series_None(self):
+        snap = self.factory.makeSnap(
+            registrant=self.person, owner=self.person,
+            distroseries=None,
+            store_series=None)
+
+        browser = self.getViewBrowser(snap, view_name="+edit", user=snap.owner)
+        self.assertIn(
+            "(unset)",
+            browser.getControl(name="field.store_distro_series").options)
+        browser.getControl(
+            name="field.store_distro_series").value = '(unset)'
+        browser.getControl("Update snap package").click()
+        self.assertEqual([], find_tags_by_class(browser.contents, "message"))
+
+        login_person(self.person)
+        self.assertThat(snap, MatchesStructure(
+            distro_series=Is(None),
+            store_series=Is(None)))
+
     def test_edit_snap_sets_date_last_modified(self):
         # Editing a snap package sets the date_last_modified property.
         date_created = datetime(2000, 1, 1, tzinfo=pytz.UTC)
diff --git a/lib/lp/snappy/configure.zcml b/lib/lp/snappy/configure.zcml
index d062ee1..a16c664 100644
--- a/lib/lp/snappy/configure.zcml
+++ b/lib/lp/snappy/configure.zcml
@@ -127,6 +127,12 @@
             interface="lp.snappy.interfaces.snappyseries.ISnappyDistroSeries" />
     </class>
 
+    <!-- SyntheticSnappyDistroSeries -->
+    <class class="lp.snappy.vocabularies.SyntheticSnappyDistroSeries">
+        <allow
+            interface="lp.snappy.interfaces.snappyseries.ISnappyDistroSeries" />
+    </class>
+
     <!-- SnappySeriesSet -->
     <securedutility
         class="lp.snappy.model.snappyseries.SnappySeriesSet"
diff --git a/lib/lp/snappy/model/snappyseries.py b/lib/lp/snappy/model/snappyseries.py
index 2fbbcaf..c60fe3b 100644
--- a/lib/lp/snappy/model/snappyseries.py
+++ b/lib/lp/snappy/model/snappyseries.py
@@ -8,6 +8,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 __metaclass__ = type
 __all__ = [
     'SnappyDistroSeries',
+    'SnappyDistroSeriesMixin',
     'SnappySeries',
     ]
 
@@ -164,8 +165,36 @@ class SnappySeries(Storm):
             get_property_cache(self)._can_infer_distro_series = False
 
 
+class SnappyDistroSeriesMixin:
+
+    @property
+    def title(self):
+        # The conditional for SeriesStatus.CURRENT here
+        # was introduced in 2020 when CURRENT meant 16
+        # When we need to introduce a new store_series;
+        # we can initially add it as FUTURE or EXPERIMENTAL
+        # until we sort out the UI.
+
+        if self.distro_series is not None:
+            if (self.snappy_series is not None and
+                    self.snappy_series.status != SeriesStatus.CURRENT):
+                return "%s, for %s" % (
+                    self.distro_series.fullseriesname,
+                    self.snappy_series.title)
+            else:
+                return self.distro_series.fullseriesname
+        else:
+            if self.snappy_series is not None:
+                if self.snappy_series.status == SeriesStatus.CURRENT:
+                    return "Infer from snapcraft.yaml (recommended)"
+                else:
+                    return self.snappy_series.title
+            else:
+                return "(unset)"
+
+
 @implementer(ISnappyDistroSeries)
-class SnappyDistroSeries(Storm):
+class SnappyDistroSeries(Storm, SnappyDistroSeriesMixin):
     """Link table between `SnappySeries` and `DistroSeries`."""
 
     __storm_table__ = 'SnappyDistroSeries'
@@ -186,14 +215,6 @@ class SnappyDistroSeries(Storm):
         self.distro_series = distro_series
         self.preferred = preferred
 
-    @property
-    def title(self):
-        if self.distro_series is not None:
-            return "%s, for %s" % (
-                self.distro_series.fullseriesname, self.snappy_series.title)
-        else:
-            return self.snappy_series.title
-
 
 @implementer(ISnappySeriesSet)
 class SnappySeriesSet:
diff --git a/lib/lp/snappy/tests/test_snapvocabularies.py b/lib/lp/snappy/tests/test_snapvocabularies.py
new file mode 100644
index 0000000..22d90c1
--- /dev/null
+++ b/lib/lp/snappy/tests/test_snapvocabularies.py
@@ -0,0 +1,92 @@
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the snappy vocabularies."""
+
+__metaclass__ = type
+
+from lp.registry.interfaces.series import SeriesStatus
+from lp.snappy.vocabularies import SnappyDistroSeriesVocabulary
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import ZopelessDatabaseLayer
+
+
+class TestSnappyDistroSeriesVocabulary(TestCaseWithFactory):
+    """Test that the SnappyDistroSeriesVocabulary behaves as expected."""
+
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(TestSnappyDistroSeriesVocabulary, self).setUp()
+        self.vocab = SnappyDistroSeriesVocabulary()
+
+    def test_getTermByToken(self):
+        distro_series = self.factory.makeDistroSeries()
+        snappy_series = self.factory.makeSnappySeries(
+            usable_distro_series=[distro_series])
+
+        term = self.vocab.getTermByToken("%s/%s/%s" % (
+            distro_series.distribution.name,
+            distro_series.name,
+            snappy_series.name
+        ))
+        self.assertIsNotNone(term)
+
+    def test_term_structure_for_current_store_series(self):
+        distro_series = self.factory.makeUbuntuDistroSeries()
+        snappy_series = self.factory.makeSnappySeries(
+            usable_distro_series=[distro_series],
+            status=SeriesStatus.CURRENT
+        )
+
+        term = self.vocab.getTermByToken("%s/%s/%s" % (
+            distro_series.distribution.name,
+            distro_series.name,
+            snappy_series.name
+        ))
+
+        self.assertEqual(term.title, distro_series.fullseriesname)
+
+    def test_term_structure_for_older_store_series(self):
+        distro_series = self.factory.makeUbuntuDistroSeries()
+        snappy_series = self.factory.makeSnappySeries(
+            usable_distro_series=[distro_series],
+            status=SeriesStatus.SUPPORTED)
+
+        term = self.vocab.getTermByToken("%s/%s/%s" % (
+            distro_series.distribution.name,
+            distro_series.name,
+            snappy_series.name
+        ))
+
+        self.assertEqual(term.title, "%s, for %s" % (
+            distro_series.fullseriesname,
+            snappy_series.title))
+
+    def test_term_structure_for_distro_series_None(self):
+        snappy_series = self.factory.makeSnappySeries(
+            can_infer_distro_series=True)
+        term = self.vocab.getTermByToken(snappy_series.name)
+        self.assertEqual(term.title, snappy_series.title)
+
+    def test_term_structure_for_infer_from_snapcraft(self):
+        snappy_series = self.factory.makeSnappySeries(
+            can_infer_distro_series=True,
+            status=SeriesStatus.CURRENT)
+        term = self.vocab.getTermByToken(snappy_series.name)
+
+        self.assertEqual(term.title, "Infer from snapcraft.yaml (recommended)")
+        self.assertEqual(term.token, snappy_series.name)
+
+    def test_term_order_for_distro_series_None(self):
+        # entry with DistroSeries=None should be at the top of
+        # the entries list in the vocabulary
+        # in this instance it will also be the most recent one
+        # which should also be a the top
+
+        snappy_series = self.factory.makeSnappySeries(
+            can_infer_distro_series=True)
+        entries = self.vocab._entries
+
+        self.assertIsNone(entries[0].distro_series)
+        self.assertEqual(entries[0].title, snappy_series.title)
diff --git a/lib/lp/snappy/vocabularies.py b/lib/lp/snappy/vocabularies.py
index 331088a..ea88db2 100644
--- a/lib/lp/snappy/vocabularies.py
+++ b/lib/lp/snappy/vocabularies.py
@@ -1,14 +1,14 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the GNU
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the GNU
 # Affero General Public License version 3 (see the file LICENSE).
 
 """Snappy vocabularies."""
 
 from __future__ import absolute_import, print_function, unicode_literals
 
+
 __metaclass__ = type
 
 __all__ = [
-    'BuildableSnappyDistroSeriesVocabulary',
     'SnapDistroArchSeriesVocabulary',
     'SnappyDistroSeriesVocabulary',
     'SnappySeriesVocabulary',
@@ -34,15 +34,15 @@ from lp.registry.model.distribution import Distribution
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.series import ACTIVE_STATUSES
 from lp.services.database.interfaces import IStore
-from lp.services.database.stormexpr import (
-    IsDistinctFrom,
-    NullsFirst,
-    )
+from lp.services.database.stormexpr import IsDistinctFrom
+from lp.services.utils import seconds_since_epoch
 from lp.services.webapp.vocabulary import StormVocabularyBase
 from lp.snappy.interfaces.snap import ISnap
+from lp.snappy.interfaces.snappyseries import ISnappyDistroSeries
 from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
 from lp.snappy.model.snappyseries import (
     SnappyDistroSeries,
+    SnappyDistroSeriesMixin,
     SnappySeries,
     )
 from lp.soyuz.model.distroarchseries import DistroArchSeries
@@ -72,6 +72,45 @@ class SnappySeriesVocabulary(StormVocabularyBase):
     _order_by = Desc(SnappySeries.date_created)
 
 
+@implementer(ISnappyDistroSeries)
+class SyntheticSnappyDistroSeries(SnappyDistroSeriesMixin):
+    def __init__(self, snappy_series, distro_series):
+        self.snappy_series = snappy_series
+        self.distro_series = distro_series
+
+    preferred = False
+
+    def __eq__(self, other):
+        return (
+            ISnappyDistroSeries.providedBy(other) and
+            self.snappy_series == other.snappy_series and
+            self.distro_series == other.distro_series)
+
+    def __ne__(self, other):
+        return not (self == other)
+
+
+def sorting_tuple_date_created(element):
+    # we negate the conversion to epoch here of
+    # the two date_created in order to achieve
+    # descending order
+    if element.distro_series is not None:
+        if element.snappy_series is not None:
+            return (1, element.distro_series.distribution.display_name,
+                    (-seconds_since_epoch(element.distro_series.date_created)),
+                    (-seconds_since_epoch(element.snappy_series.date_created)))
+        else:
+            return (1, element.distro_series.distribution.display_name,
+                    (-seconds_since_epoch(element.distro_series.date_created)),
+                    None)
+    else:
+        if element.snappy_series is not None:
+            return (0, None, None,
+                    (-seconds_since_epoch(element.snappy_series.date_created)))
+        else:
+            return 0, None, None, None
+
+
 class SnappyDistroSeriesVocabulary(StormVocabularyBase):
     """A vocabulary for searching snappy/distro series combinations."""
 
@@ -84,25 +123,39 @@ class SnappyDistroSeriesVocabulary(StormVocabularyBase):
         LeftJoin(Distribution, DistroSeries.distributionID == Distribution.id),
         SnappySeries,
         ]
-    _clauses = [SnappyDistroSeries.snappy_series_id == SnappySeries.id]
+    _clauses = [SnappyDistroSeries.snappy_series_id == SnappySeries.id,
+                SnappySeries.status.is_in(ACTIVE_STATUSES)]
 
     @property
     def _entries(self):
-        entries = IStore(self._table).using(*self._origin).find(
-            self._table, *self._clauses)
-        return entries.order_by(
-            NullsFirst(Distribution.display_name),
-            Desc(DistroSeries.date_created),
-            Desc(SnappySeries.date_created))
+        entries = list(IStore(self._table).using(*self._origin).find(
+            self._table, *self._clauses))
+
+        if (ISnap.providedBy(self.context) and not
+                any(entry.snappy_series == self.context.store_series
+                    and entry.distro_series == self.context.distro_series
+                    for entry in entries)):
+            entries.append(SyntheticSnappyDistroSeries(
+                self.context.store_series, self.context.distro_series))
+        return sorted(entries, key=sorting_tuple_date_created)
 
     def toTerm(self, obj):
         """See `IVocabulary`."""
-        if obj.distro_series is None:
-            token = obj.snappy_series.name
+        if obj.snappy_series is not None:
+            if obj.distro_series is None:
+                token = obj.snappy_series.name
+            else:
+                token = "%s/%s/%s" % (
+                    obj.distro_series.distribution.name,
+                    obj.distro_series.name,
+                    obj.snappy_series.name)
         else:
-            token = "%s/%s/%s" % (
-                obj.distro_series.distribution.name, obj.distro_series.name,
-                obj.snappy_series.name)
+            if obj.distro_series is None:
+                token = "(unset)"
+            else:
+                token = "%s/%s" % (
+                        obj.distro_series.distribution.name,
+                        obj.distro_series.name)
         return SimpleTerm(obj, token, obj.title)
 
     def __contains__(self, value):
@@ -117,11 +170,17 @@ class SnappyDistroSeriesVocabulary(StormVocabularyBase):
 
     def getTermByToken(self, token):
         """See `IVocabularyTokenized`."""
+        if token == "(unset)":
+            return self.toTerm(SyntheticSnappyDistroSeries(None, None))
         if "/" in token:
-            try:
+            bits = token.split("/", 2)
+            if len(bits) == 2:
+                distribution_name, distro_series_name = bits
+                snappy_series_name = None
+            elif len(bits) == 3:
                 distribution_name, distro_series_name, snappy_series_name = (
-                    token.split("/", 2))
-            except ValueError:
+                    bits)
+            else:
                 raise LookupError(token)
         else:
             distribution_name = None
@@ -133,17 +192,28 @@ class SnappyDistroSeriesVocabulary(StormVocabularyBase):
             Not(IsDistinctFrom(DistroSeries.name, distro_series_name)),
             SnappySeries.name == snappy_series_name,
             *self._clauses).one()
+
+        if entry is None and ISnap.providedBy(self.context):
+            if self.context.store_series is None:
+                context_store_series_name = None
+            else:
+                context_store_series_name = self.context.store_series.name
+            if self.context.distro_series is not None:
+                context_distribution_name = (
+                    self.context.distro_series.distribution.name)
+                context_distro_series_name = self.context.distro_series.name
+            else:
+                context_distribution_name = None
+                context_distro_series_name = None
+            if (context_distribution_name == distribution_name and
+                    context_distro_series_name == distro_series_name and
+                    context_store_series_name == snappy_series_name):
+                entry = SyntheticSnappyDistroSeries(
+                    self.context.store_series, self.context.distro_series)
         if entry is None:
             raise LookupError(token)
-        return self.toTerm(entry)
 
-
-class BuildableSnappyDistroSeriesVocabulary(SnappyDistroSeriesVocabulary):
-    """A vocabulary for searching active snappy/distro series combinations."""
-
-    _clauses = SnappyDistroSeriesVocabulary._clauses + [
-        SnappySeries.status.is_in(ACTIVE_STATUSES),
-        ]
+        return self.toTerm(entry)
 
 
 @implementer(IJSONPublishable)
diff --git a/lib/lp/snappy/vocabularies.zcml b/lib/lp/snappy/vocabularies.zcml
index 8f34394..4df0102 100644
--- a/lib/lp/snappy/vocabularies.zcml
+++ b/lib/lp/snappy/vocabularies.zcml
@@ -37,17 +37,6 @@
         <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary" />
     </class>
 
-    <securedutility
-        name="BuildableSnappyDistroSeries"
-        component="lp.snappy.vocabularies.BuildableSnappyDistroSeriesVocabulary"
-        provides="zope.schema.interfaces.IVocabularyFactory">
-        <allow interface="zope.schema.interfaces.IVocabularyFactory" />
-    </securedutility>
-
-    <class class="lp.snappy.vocabularies.BuildableSnappyDistroSeriesVocabulary">
-        <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary" />
-    </class>
-
     <class class="lp.snappy.vocabularies.SnapStoreChannel">
         <allow interface="zope.schema.interfaces.ITitledTokenizedTerm
                           lazr.restful.interfaces.IJSONPublishable" />