launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/livefs-delete into lp:launchpad.

Commit message:
Allow deleting live filesystems, provided that they have no builds.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1452449 in Launchpad itself: "Please allow me to delete livefs definitions"
  https://bugs.launchpad.net/launchpad/+bug/1452449

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/livefs-delete/+merge/258481

Allow deleting live filesystems, provided that they have no builds.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/livefs-delete into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/configure.zcml'
--- lib/lp/soyuz/browser/configure.zcml	2015-04-20 11:24:21 +0000
+++ lib/lp/soyuz/browser/configure.zcml	2015-05-07 10:56:40 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -723,6 +723,12 @@
         permission="launchpad.Edit"
         name="+edit"
         template="../../app/templates/generic-edit.pt"/>
+    <browser:page
+        for="lp.soyuz.interfaces.livefs.ILiveFS"
+        class="lp.soyuz.browser.livefs.LiveFSDeleteView"
+        permission="launchpad.Edit"
+        name="+delete"
+        template="../templates/livefs-delete.pt"/>
     <adapter
         provides="lp.services.webapp.interfaces.IBreadcrumb"
         for="lp.soyuz.interfaces.livefs.ILiveFS"

=== modified file 'lib/lp/soyuz/browser/livefs.py'
--- lib/lp/soyuz/browser/livefs.py	2014-11-16 09:49:59 +0000
+++ lib/lp/soyuz/browser/livefs.py	2015-05-07 10:56:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """LiveFS views."""
@@ -6,6 +6,7 @@
 __metaclass__ = type
 __all__ = [
     'LiveFSAddView',
+    'LiveFSDeleteView',
     'LiveFSEditView',
     'LiveFSNavigation',
     'LiveFSNavigationMenu',
@@ -94,7 +95,7 @@
 
     facet = 'overview'
 
-    links = ('admin', 'edit',)
+    links = ('admin', 'delete', 'edit')
 
     @enabled_with_permission('launchpad.Admin')
     def admin(self):
@@ -104,6 +105,10 @@
     def edit(self):
         return Link('+edit', 'Edit live filesystem', icon='edit')
 
+    @enabled_with_permission('launchpad.Edit')
+    def delete(self):
+        return Link('+delete', 'Delete live filesystem', icon='trash-icon')
+
 
 class LiveFSView(LaunchpadView):
     """Default view of a LiveFS."""
@@ -329,3 +334,27 @@
                             distro_series.displayname, owner.displayname))
             except NoSuchLiveFS:
                 pass
+
+
+class LiveFSDeleteView(BaseLiveFSEditView):
+    """View for deleting live filesystems."""
+
+    @property
+    def title(self):
+        return 'Delete %s live filesystem' % self.context.name
+
+    label = title
+
+    field_names = []
+
+    @property
+    def has_builds(self):
+        return not self.context.builds.is_empty()
+
+    @action('Delete live filesystem', name='delete')
+    def delete_action(self, action, data):
+        owner = self.context.owner
+        self.context.destroySelf()
+        # XXX cjwatson 2015-05-07 bug=1332479: This should go to
+        # Person:+livefs once that exists.
+        self.next_url = canonical_url(owner)

=== modified file 'lib/lp/soyuz/browser/tests/test_livefs.py'
--- lib/lp/soyuz/browser/tests/test_livefs.py	2015-04-20 09:48:57 +0000
+++ lib/lp/soyuz/browser/tests/test_livefs.py	2015-05-07 10:56:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test live filesystem views."""
@@ -15,6 +15,7 @@
 from mechanize import LinkNotFoundError
 import pytz
 from zope.component import getUtility
+from zope.publisher.interfaces import NotFound
 from zope.security.interfaces import Unauthorized
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -316,6 +317,56 @@
             extract_text(find_tags_by_class(browser.contents, "message")[1]))
 
 
+class TestLiveFSDeleteView(BrowserTestCase):
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestLiveFSDeleteView, self).setUp()
+        self.useFixture(FeatureFixture({LIVEFS_FEATURE_FLAG: u"on"}))
+        self.person = self.factory.makePerson(
+            name="test-person", displayname="Test Person")
+
+    def test_unauthorized(self):
+        # A user without edit access cannot delete a live filesystem.
+        self.useFixture(FakeLogger())
+        livefs = self.factory.makeLiveFS(
+            registrant=self.person, owner=self.person)
+        livefs_url = canonical_url(livefs)
+        other_person = self.factory.makePerson()
+        browser = self.getViewBrowser(livefs, user=other_person)
+        self.assertRaises(
+            LinkNotFoundError, browser.getLink, "Delete live filesystem")
+        self.assertRaises(
+            Unauthorized, self.getUserBrowser, livefs_url + "/+delete",
+            user=other_person)
+
+    def test_delete_livefs_without_builds(self):
+        # A live filesystem without builds can be deleted.
+        self.useFixture(FakeLogger())
+        livefs = self.factory.makeLiveFS(
+            registrant=self.person, owner=self.person)
+        livefs_url = canonical_url(livefs)
+        owner_url = canonical_url(self.person)
+        browser = self.getViewBrowser(livefs, user=self.person)
+        browser.getLink("Delete live filesystem").click()
+        browser.getControl("Delete live filesystem").click()
+        self.assertEqual(owner_url, browser.url)
+        self.assertRaises(NotFound, browser.open, livefs_url)
+
+    def test_delete_livefs_with_builds(self):
+        # A live filesystem without builds cannot be deleted.
+        livefs = self.factory.makeLiveFS(
+            registrant=self.person, owner=self.person)
+        self.factory.makeLiveFSBuild(livefs=livefs)
+        browser = self.getViewBrowser(livefs, user=self.person)
+        browser.getLink("Delete live filesystem").click()
+        self.assertIn(
+            "This live filesystem cannot be deleted", browser.contents)
+        self.assertRaises(
+            LookupError, browser.getControl, "Delete live filesystem")
+
+
 class TestLiveFSView(BrowserTestCase):
 
     layer = LaunchpadFunctionalLayer

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2015-04-20 11:27:54 +0000
+++ lib/lp/soyuz/configure.zcml	2015-05-07 10:56:40 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -963,6 +963,7 @@
                        .interfaces.livefs.ILiveFSAdminAttributes"/>
         <require
             permission="launchpad.Edit"
+            interface=".interfaces.livefs.ILiveFSEdit"
             set_schema=".interfaces.livefs.ILiveFSEditableAttributes"
             set_attributes="date_last_modified"/>
         <require

=== modified file 'lib/lp/soyuz/interfaces/livefs.py'
--- lib/lp/soyuz/interfaces/livefs.py	2014-06-18 18:29:13 +0000
+++ lib/lp/soyuz/interfaces/livefs.py	2015-05-07 10:56:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Live filesystem interfaces."""
@@ -6,6 +6,7 @@
 __metaclass__ = type
 
 __all__ = [
+    'CannotDeleteLiveFS',
     'DuplicateLiveFSName',
     'ILiveFS',
     'ILiveFSEditableAttributes',
@@ -28,6 +29,7 @@
     error_status,
     export_as_webservice_collection,
     export_as_webservice_entry,
+    export_destructor_operation,
     export_factory_operation,
     export_read_operation,
     exported,
@@ -131,6 +133,11 @@
     _message_prefix = "No such live filesystem with this owner/distroseries"
 
 
+@error_status(httplib.BAD_REQUEST)
+class CannotDeleteLiveFS(Exception):
+    """This live filesystem cannot be deleted."""
+
+
 class ILiveFSView(Interface):
     """`ILiveFS` attributes that require launchpad.View permission."""
 
@@ -200,6 +207,15 @@
         value_type=Reference(schema=Interface), readonly=True)))
 
 
+class ILiveFSEdit(Interface):
+    """`ILiveFS` methods that require launchpad.Edit permission."""
+
+    @export_destructor_operation()
+    @operation_for_version("devel")
+    def destroySelf():
+        """Delete this live filesystem, provided that it has no builds."""
+
+
 class ILiveFSEditableAttributes(IHasOwner):
     """`ILiveFS` attributes that can be edited.
 
@@ -240,7 +256,9 @@
             "Only build this live filesystem image on virtual builders.")))
 
 
-class ILiveFS(ILiveFSView, ILiveFSEditableAttributes, ILiveFSAdminAttributes):
+class ILiveFS(
+    ILiveFSView, ILiveFSEdit, ILiveFSEditableAttributes,
+    ILiveFSAdminAttributes):
     """A buildable live filesystem image."""
 
     # XXX cjwatson 2014-05-06 bug=760849: "beta" is a lie to get WADL

=== modified file 'lib/lp/soyuz/model/livefs.py'
--- lib/lp/soyuz/model/livefs.py	2014-06-18 18:29:13 +0000
+++ lib/lp/soyuz/model/livefs.py	2015-05-07 10:56:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -52,6 +52,7 @@
 from lp.services.webapp.interfaces import ILaunchBag
 from lp.soyuz.interfaces.archive import ArchiveDisabled
 from lp.soyuz.interfaces.livefs import (
+    CannotDeleteLiveFS,
     DuplicateLiveFSName,
     ILiveFS,
     ILiveFSSet,
@@ -210,6 +211,13 @@
         order_by = Desc(LiveFSBuild.id)
         return self._getBuilds(filter_term, order_by)
 
+    def destroySelf(self):
+        """See `ILiveFS`."""
+        if not self.builds.is_empty():
+            raise CannotDeleteLiveFS(
+                "Cannot delete a live filesystem with builds.")
+        IStore(LiveFS).remove(self)
+
 
 class LiveFSSet:
     """See `ILiveFSSet`."""

=== added file 'lib/lp/soyuz/templates/livefs-delete.pt'
--- lib/lp/soyuz/templates/livefs-delete.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/templates/livefs-delete.pt	2015-05-07 10:56:40 +0000
@@ -0,0 +1,22 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml";
+  xmlns:tal="http://xml.zope.org/namespaces/tal";
+  xmlns:metal="http://xml.zope.org/namespaces/metal";
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+  metal:use-macro="view/macro:page/main_only"
+  i18n:domain="launchpad">
+<body>
+
+  <div metal:fill-slot="main">
+    <tal:has-builds condition="view/has_builds">
+      This live filesystem cannot be deleted as builds have been created for
+      it.
+    </tal:has-builds>
+
+    <tal:has-no-builds condition="not: view/has_builds">
+      <div metal:use-macro="context/@@launchpad_form/form"/>
+    </tal:has-no-builds>
+  </div>
+
+</body>
+</html>

=== modified file 'lib/lp/soyuz/tests/test_livefs.py'
--- lib/lp/soyuz/tests/test_livefs.py	2014-09-01 03:45:08 +0000
+++ lib/lp/soyuz/tests/test_livefs.py	2015-05-07 10:56:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test live filesystems."""
@@ -32,6 +32,7 @@
 from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.interfaces import OAuthPermission
 from lp.soyuz.interfaces.livefs import (
+    CannotDeleteLiveFS,
     ILiveFS,
     ILiveFSSet,
     ILiveFSView,
@@ -83,7 +84,8 @@
     def test_implements_interfaces(self):
         # LiveFS implements ILiveFS.
         livefs = self.factory.makeLiveFS()
-        self.assertProvides(livefs, ILiveFS)
+        with person_logged_in(livefs.owner):
+            self.assertProvides(livefs, ILiveFS)
 
     def test_avoids_problematic_snapshots(self):
         self.assertThat(
@@ -245,6 +247,35 @@
         self.assertEqual([], list(livefs.builds))
         self.assertEqual([], list(livefs.pending_builds))
 
+    def test_delete_without_builds(self):
+        # A live filesystem with no builds can be deleted.
+        owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        livefs = self.factory.makeLiveFS(
+            registrant=owner, owner=owner, distroseries=distroseries,
+            name=u"condemned")
+        self.assertTrue(
+            getUtility(ILiveFSSet).exists(owner, distroseries, u"condemned"))
+        with person_logged_in(livefs.owner):
+            livefs.destroySelf()
+        self.assertFalse(
+            getUtility(ILiveFSSet).exists(owner, distroseries, u"condemned"))
+
+    def test_delete_with_builds(self):
+        # A live filesystem with builds cannot be deleted.
+        owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        livefs = self.factory.makeLiveFS(
+            registrant=owner, owner=owner, distroseries=distroseries,
+            name=u"condemned")
+        self.factory.makeLiveFSBuild(livefs=livefs)
+        self.assertTrue(
+            getUtility(ILiveFSSet).exists(owner, distroseries, u"condemned"))
+        with person_logged_in(livefs.owner):
+            self.assertRaises(CannotDeleteLiveFS, livefs.destroySelf)
+        self.assertTrue(
+            getUtility(ILiveFSSet).exists(owner, distroseries, u"condemned"))
+
 
 class TestLiveFSSet(TestCaseWithFactory):