launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/restore-new-snap-links into lp:launchpad.

Commit message:
Restore "Create snap package" link for contexts with no snap packages.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1603969 in Launchpad itself: "No way to create snaps for contexts with no snaps"
  https://bugs.launchpad.net/launchpad/+bug/1603969

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/restore-new-snap-links/+merge/300326

Restore "Create snap package" link for contexts with no snap packages.  This was lost by mistake in https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562 - "bool(getFeatureFlag(SNAP_FEATURE_FLAG)) or not self.snaps.is_empty()" should have been simplified to True rather than to "not self.snaps.is_empty()", since that feature flag is now effectively always set.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/restore-new-snap-links into lp:launchpad.

=== modified file 'lib/lp/snappy/browser/tests/test_hassnaps.py'
--- lib/lp/snappy/browser/tests/test_hassnaps.py	2016-06-28 21:10:18 +0000
+++ lib/lp/snappy/browser/tests/test_hassnaps.py	2016-07-18 13:53:41 +0000
@@ -1,23 +1,72 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test views for objects that have snap packages."""
 
 __metaclass__ = type
 
+import soupmatchers
+from testscenarios import (
+    load_tests_apply_scenarios,
+    WithScenarios,
+    )
+
+from lp.code.interfaces.branch import IBranch
+from lp.code.interfaces.githosting import IGitHostingClient
+from lp.code.interfaces.gitrepository import IGitRepository
 from lp.services.webapp import canonical_url
 from lp.testing import TestCaseWithFactory
-from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import ZopeUtilityFixture
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadFunctionalLayer,
+    )
 from lp.testing.views import create_initialized_view
 
 
-class TestRelatedSnapsMixin:
+def make_branch(test_case):
+    return test_case.factory.makeAnyBranch()
+
+
+def make_git_repository(test_case):
+    return test_case.factory.makeGitRepository()
+
+
+def make_git_ref(test_case):
+    return test_case.factory.makeGitRefs()[0]
+
+
+class TestHasSnapsView(WithScenarios, TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
+    scenarios = [
+        ("Branch", {
+            "context_type": "branch",
+            "context_factory": make_branch,
+            }),
+        ("GitRepository", {
+            "context_type": "repository",
+            "context_factory": make_git_repository,
+            }),
+        ("GitRef", {
+            "context_type": "branch",
+            "context_factory": make_git_ref,
+            }),
+        ]
+
+    def makeSnap(self, context):
+        if IBranch.providedBy(context):
+            return self.factory.makeSnap(branch=context)
+        else:
+            if IGitRepository.providedBy(context):
+                [context] = self.factory.makeGitRefs(repository=context)
+            return self.factory.makeSnap(git_ref=context)
+
     def test_snaps_link_no_snaps(self):
         # An object with no snap packages does not show a snap packages link.
-        context = self.makeContext()
+        context = self.context_factory(self)
         view = create_initialized_view(context, "+index")
         self.assertEqual(
             "No snap packages using this %s." % self.context_type,
@@ -25,7 +74,7 @@
 
     def test_snaps_link_one_snap(self):
         # An object with one snap package shows a link to that snap package.
-        context = self.makeContext()
+        context = self.context_factory(self)
         snap = self.makeSnap(context)
         view = create_initialized_view(context, "+index")
         expected_link = (
@@ -35,7 +84,7 @@
 
     def test_snaps_link_more_snaps(self):
         # An object with more than one snap package shows a link to a listing.
-        context = self.makeContext()
+        context = self.context_factory(self)
         self.makeSnap(context)
         self.makeSnap(context)
         view = create_initialized_view(context, "+index")
@@ -45,36 +94,56 @@
         self.assertEqual(expected_link, view.snaps_link)
 
 
-class TestRelatedSnapsBranch(TestRelatedSnapsMixin, TestCaseWithFactory):
-
-    context_type = "branch"
-
-    def makeContext(self):
-        return self.factory.makeAnyBranch()
-
-    def makeSnap(self, context):
-        return self.factory.makeSnap(branch=context)
-
-
-class TestRelatedSnapsGitRepository(
-    TestRelatedSnapsMixin, TestCaseWithFactory):
-
-    context_type = "repository"
-
-    def makeContext(self):
-        return self.factory.makeGitRepository()
-
-    def makeSnap(self, context):
-        [ref] = self.factory.makeGitRefs(repository=context)
-        return self.factory.makeSnap(git_ref=ref)
-
-
-class TestRelatedSnapsGitRef(TestRelatedSnapsMixin, TestCaseWithFactory):
-
-    context_type = "branch"
-
-    def makeContext(self):
-        return self.factory.makeGitRefs()[0]
-
-    def makeSnap(self, context):
-        return self.factory.makeSnap(git_ref=context)
+class TestHasSnapsMenu(WithScenarios, TestCaseWithFactory):
+
+    needs_hosting_client = False
+
+    scenarios = [
+        ("Branch", {
+            "layer": DatabaseFunctionalLayer,
+            "context_factory": make_branch,
+            }),
+        ("GitRef", {
+            "layer": LaunchpadFunctionalLayer,
+            "context_factory": make_git_ref,
+            "needs_hosting_client": True,
+            }),
+        ]
+
+    def setUp(self):
+        super(TestHasSnapsMenu, self).setUp()
+        if self.needs_hosting_client:
+            hosting_client = FakeMethod()
+            hosting_client.getLog = FakeMethod(result=[])
+            self.useFixture(
+                ZopeUtilityFixture(hosting_client, IGitHostingClient))
+
+    def makeSnap(self, context):
+        if IBranch.providedBy(context):
+            return self.factory.makeSnap(branch=context)
+        else:
+            return self.factory.makeSnap(git_ref=context)
+
+    def test_creation_link_no_snaps(self):
+        # An object with no snap packages shows a creation link.
+        context = self.context_factory(self)
+        view = create_initialized_view(context, "+index")
+        new_snap_url = canonical_url(context, view_name="+new-snap")
+        self.assertThat(view(), soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                "creation link", "a", attrs={"href": new_snap_url},
+                text="Create snap package")))
+
+    def test_creation_link_snaps(self):
+        # An object with snap packages shows a creation link.
+        context = self.context_factory(self)
+        self.makeSnap(context)
+        view = create_initialized_view(context, "+index")
+        new_snap_url = canonical_url(context, view_name="+new-snap")
+        self.assertThat(view(), soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                "creation link", "a", attrs={"href": new_snap_url},
+                text="Create snap package")))
+
+
+load_tests = load_tests_apply_scenarios

=== modified file 'lib/lp/snappy/templates/snap-macros.pt'
--- lib/lp/snappy/templates/snap-macros.pt	2016-06-30 17:25:25 +0000
+++ lib/lp/snappy/templates/snap-macros.pt	2016-07-18 13:53:41 +0000
@@ -6,7 +6,6 @@
 
 <div
   metal:define-macro="related-snaps"
-  tal:condition="not: view/snaps/is_empty"
   tal:define="context_menu context/menu:context"
   id="related-snaps">