launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:livefs-webhooks-manage-link into launchpad:master.

Commit message:
Add a "Manage webhooks" navigation link to LiveFS

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

I added a generic webhook test to ensure that this isn't missed for new webhook targets in future, although this turned out to be unexpectedly difficult due to details of how navigation menus work.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:livefs-webhooks-manage-link into launchpad:master.

diff --git a/lib/lp/services/webhooks/tests/test_browser.py b/lib/lp/services/webhooks/tests/test_browser.py
index 13adc13..39021bd 100644
--- a/lib/lp/services/webhooks/tests/test_browser.py
+++ b/lib/lp/services/webhooks/tests/test_browser.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for Webhook views."""
@@ -14,8 +14,10 @@ from testtools.matchers import (
     Not,
     )
 import transaction
+from zope.component import getUtility
 
 from lp.services.features.testing import FeatureFixture
+from lp.services.webapp.interfaces import IPlacelessAuthUtility
 from lp.services.webapp.publisher import canonical_url
 from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
 from lp.soyuz.interfaces.livefs import (
@@ -140,10 +142,23 @@ class WebhookTargetViewTestHelpers:
         login_person(self.owner)
 
     def makeView(self, name, **kwargs):
-        view = create_view(self.target, name, principal=self.owner, **kwargs)
+        # XXX cjwatson 2020-02-06: We need to give the view a
+        # LaunchpadPrincipal rather than just a person, since otherwise bits
+        # of the navigation menu machinery try to use the scope_url
+        # attribute on the principal and fail.  This should probably be done
+        # in create_view instead, but that approach needs care to avoid
+        # adding an extra query to tests that might be sensitive to that.
+        principal = getUtility(IPlacelessAuthUtility).getPrincipal(
+            self.owner.accountID)
+        view = create_view(
+            self.target, name, principal=principal, current_request=True,
+            **kwargs)
         # To test the breadcrumbs we need a correct traversal stack.
         view.request.traversed_objects = (
             self.getTraversalStack(self.target) + [view])
+        # The navigation menu machinery needs this to find the view from the
+        # request.
+        view.request._last_obj_traversed = view
         view.initialize()
         return view
 
@@ -166,6 +181,17 @@ class TestWebhooksViewBase(WebhookTargetViewTestHelpers):
             for hook in hooks]
         return link_matchers
 
+    def test_navigation_from_context(self):
+        # The context object's index page shows a "Manage webhooks" link.
+        self.assertThat(
+            self.makeView("+index")(),
+            soupmatchers.HTMLContains(
+                soupmatchers.Tag(
+                    "manage webhooks link", "a", text="Manage webhooks",
+                    attrs={"href": canonical_url(
+                        self.target, view_name="+webhooks"),
+                        })))
+
     def test_empty(self):
         # The table isn't shown if there are no webhooks yet.
         self.assertThat(
diff --git a/lib/lp/soyuz/browser/livefs.py b/lib/lp/soyuz/browser/livefs.py
index e2659ca..127df0b 100644
--- a/lib/lp/soyuz/browser/livefs.py
+++ b/lib/lp/soyuz/browser/livefs.py
@@ -1,4 +1,4 @@
-# Copyright 2014-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """LiveFS views."""
@@ -61,6 +61,7 @@ from lp.soyuz.interfaces.livefs import (
     ILiveFS,
     ILiveFSSet,
     LIVEFS_FEATURE_FLAG,
+    LIVEFS_WEBHOOKS_FEATURE_FLAG,
     LiveFSFeatureDisabled,
     NoSuchLiveFS,
     )
@@ -95,7 +96,7 @@ class LiveFSNavigationMenu(NavigationMenu):
 
     facet = 'overview'
 
-    links = ('admin', 'delete', 'edit')
+    links = ('admin', 'edit', 'webhooks', 'delete')
 
     @enabled_with_permission('launchpad.Admin')
     def admin(self):
@@ -106,6 +107,12 @@ class LiveFSNavigationMenu(NavigationMenu):
         return Link('+edit', 'Edit live filesystem', icon='edit')
 
     @enabled_with_permission('launchpad.Edit')
+    def webhooks(self):
+        return Link(
+            '+webhooks', 'Manage webhooks', icon='edit',
+            enabled=bool(getFeatureFlag(LIVEFS_WEBHOOKS_FEATURE_FLAG)))
+
+    @enabled_with_permission('launchpad.Edit')
     def delete(self):
         return Link('+delete', 'Delete live filesystem', icon='trash-icon')