launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-repository-delete-ui into lp:launchpad with lp:~cjwatson/launchpad/git-repository-delete as a prerequisite.

Commit message:
Add a new GitRepository:+delete view, linked from a new edit menu on GitRepository.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1456583 in Launchpad itself: "Can't delete Git repositories"
  https://bugs.launchpad.net/launchpad/+bug/1456583

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-repository-delete-ui/+merge/259488

Add a new GitRepository:+delete view, linked from a new edit menu on GitRepository.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-repository-delete-ui into lp:launchpad.

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2015-05-12 17:20:22 +0000
+++ lib/lp/code/browser/configure.zcml	2015-05-19 11:36:43 +0000
@@ -753,7 +753,8 @@
     <browser:menus
         module="lp.code.browser.gitrepository"
         classes="
-            GitRepositoryContextMenu"/>
+            GitRepositoryContextMenu
+            GitRepositoryEditMenu"/>
     <browser:pages
         for="lp.code.interfaces.gitrepository.IGitRepository"
         class="lp.code.browser.gitrepository.GitRepositoryView"
@@ -781,6 +782,12 @@
         template="../templates/gitrepository-portlet-subscribers-content.pt"/>
     <browser:page
         for="lp.code.interfaces.gitrepository.IGitRepository"
+        class="lp.code.browser.gitrepository.GitRepositoryDeletionView"
+        permission="launchpad.Edit"
+        name="+delete"
+        template="../templates/gitrepository-delete.pt"/>
+    <browser:page
+        for="lp.code.interfaces.gitrepository.IGitRepository"
         class="lp.code.browser.gitsubscription.GitSubscriptionAddView"
         permission="launchpad.AnyPerson"
         name="+subscribe"

=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py	2015-05-01 13:18:54 +0000
+++ lib/lp/code/browser/gitrepository.py	2015-05-19 11:36:43 +0000
@@ -9,6 +9,8 @@
     'GitRefBatchNavigator',
     'GitRepositoryBreadcrumb',
     'GitRepositoryContextMenu',
+    'GitRepositoryDeletionView',
+    'GitRepositoryEditMenu',
     'GitRepositoryNavigation',
     'GitRepositoryURL',
     'GitRepositoryView',
@@ -18,16 +20,23 @@
 from zope.interface import implements
 
 from lp.app.browser.informationtype import InformationTypePortletMixin
+from lp.app.browser.launchpadform import (
+    action,
+    LaunchpadFormView,
+    )
 from lp.app.errors import NotFoundError
 from lp.code.interfaces.gitref import IGitRefBatchNavigator
 from lp.code.interfaces.gitrepository import IGitRepository
 from lp.services.config import config
+from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
+    canonical_url,
     ContextMenu,
     enabled_with_permission,
     LaunchpadView,
     Link,
     Navigation,
+    NavigationMenu,
     stepto,
     )
 from lp.services.webapp.authorization import (
@@ -80,6 +89,20 @@
         raise NotFoundError
 
 
+class GitRepositoryEditMenu(NavigationMenu):
+    """Edit menu for `IGitRepository`."""
+
+    usedfor = IGitRepository
+    facet = "branches"
+    title = "Edit Git repository"
+    links = ["delete"]
+
+    @enabled_with_permission("launchpad.Edit")
+    def delete(self):
+        text = "Delete repository"
+        return Link("+delete", text, icon="trash-icon")
+
+
 class GitRepositoryContextMenu(ContextMenu):
     """Context menu for `IGitRepository`."""
 
@@ -165,3 +188,73 @@
     def branches(self):
         """All branches in this repository, sorted for display."""
         return GitRefBatchNavigator(self, self.context)
+
+
+class GitRepositoryDeletionView(LaunchpadFormView):
+
+    schema = IGitRepository
+    field_names = []
+
+    @property
+    def page_title(self):
+        return "Delete repository %s" % self.context.display_name
+
+    label = page_title
+
+    @cachedproperty
+    def display_deletion_requirements(self):
+        """Normal deletion requirements, indication of permissions.
+
+        :return: A list of tuples of (item, action, reason, allowed)
+        """
+        reqs = []
+        for item, (action, reason) in (
+                self.context.getDeletionRequirements().iteritems()):
+            allowed = check_permission("launchpad.Edit", item)
+            reqs.append((item, action, reason, allowed))
+        return reqs
+
+    def all_permitted(self):
+        """Return True if all deletion requirements are permitted, else False.
+
+        Uses display_deletion_requirements as its source data.
+        """
+        return len([item for item, action, reason, allowed in
+            self.display_deletion_requirements if not allowed]) == 0
+
+    @action("Delete", name="delete_repository",
+            condition=lambda x, _: x.all_permitted())
+    def delete_repository_action(self, action, data):
+        repository = self.context
+        if self.all_permitted():
+            # Since the user is going to delete the repository, we need to
+            # have somewhere valid to send them next.
+            self.next_url = canonical_url(repository.target)
+            message = "Repository %s deleted." % repository.unique_name
+            self.context.destroySelf(break_references=True)
+            self.request.response.addNotification(message)
+        else:
+            self.request.response.addNotification(
+                "This repository cannot be deleted.")
+            self.next_url = canonical_url(repository)
+
+    @property
+    def repository_deletion_actions(self):
+        """Return the repository deletion actions as a ZPT-friendly dict.
+
+        The keys are "delete" and "alter"; the values are dicts of
+        "item", "reason" and "allowed".
+        """
+        row_dict = {"delete": [], "alter": []}
+        for item, action, reason, allowed in (
+            self.display_deletion_requirements):
+            row = {"item": item,
+                   "reason": reason,
+                   "allowed": allowed,
+                  }
+            row_dict[action].append(row)
+        return row_dict
+
+    @property
+    def cancel_url(self):
+        return canonical_url(self.context)

=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py	2015-05-14 13:57:51 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py	2015-05-19 11:36:43 +0000
@@ -6,11 +6,15 @@
 __metaclass__ = type
 
 from datetime import datetime
+import doctest
 
 from BeautifulSoup import BeautifulSoup
 from fixtures import FakeLogger
 import pytz
-from testtools.matchers import Equals
+from testtools.matchers import (
+    DocTestMatches,
+    Equals,
+    )
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
 from zope.security.proxy import removeSecurityProxy
@@ -32,6 +36,7 @@
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import (
+    get_feedback_messages,
     setupBrowser,
     setupBrowserForUser,
     )
@@ -179,3 +184,62 @@
         recorder1, recorder2 = record_two_runs(
             lambda: self.getMainText(repository, "+index"), create_ref, 10)
         self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
+
+
+class TestGitRepositoryDeletionView(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_repository_has_delete_link(self):
+        # A newly-created repository has a "Delete repository" link.
+        repository = self.factory.makeGitRepository()
+        delete_url = canonical_url(
+            repository, view_name="+delete", rootsite="code")
+        browser = self.getViewBrowser(
+            repository, "+index", rootsite="code", user=repository.owner)
+        delete_link = browser.getLink("Delete repository")
+        self.assertEqual(delete_url, delete_link.url)
+
+    def test_warning_message(self):
+        # The deletion view informs the user what will happen if they delete
+        # the repository.
+        repository = self.factory.makeGitRepository()
+        name = repository.display_name
+        text = self.getMainText(
+            repository, "+delete", rootsite="code", user=repository.owner)
+        self.assertThat(
+            text, DocTestMatches(
+                "Delete repository %s ...\n"
+                "Repository deletion is permanent.\n"
+                "or Cancel" % name,
+                flags=(doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS)))
+
+    def test_next_url(self):
+        # Deleting a repository takes the user back to the code listing for
+        # the target, and shows a notification message.
+        project = self.factory.makeProduct()
+        project_url = canonical_url(project, rootsite="code")
+        repository = self.factory.makeGitRepository(target=project)
+        name = repository.unique_name
+        browser = self.getViewBrowser(
+            repository, "+delete", rootsite="code", user=repository.owner)
+        browser.getControl("Delete").click()
+        self.assertEqual(project_url, browser.url)
+        self.assertEqual(
+            ["Repository %s deleted." % name],
+            get_feedback_messages(browser.contents))
+
+    def test_next_url_personal(self):
+        # Deleting a personal repository takes the user back to the code
+        # listing for the owner, and shows a notification message.
+        owner = self.factory.makePerson()
+        owner_url = canonical_url(owner, rootsite="code")
+        repository = self.factory.makeGitRepository(owner=owner, target=owner)
+        name = repository.unique_name
+        browser = self.getViewBrowser(
+            repository, "+delete", rootsite="code", user=repository.owner)
+        browser.getControl("Delete").click()
+        self.assertEqual(owner_url, browser.url)
+        self.assertEqual(
+            ["Repository %s deleted." % name],
+            get_feedback_messages(browser.contents))

=== added file 'lib/lp/code/templates/gitrepository-delete.pt'
--- lib/lp/code/templates/gitrepository-delete.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/gitrepository-delete.pt	2015-05-19 11:36:43 +0000
@@ -0,0 +1,50 @@
+<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:deletelist condition="view/repository_deletion_actions/delete">
+    The following items must be <em>deleted</em>:
+    <ul id="deletion-items">
+      <tal:actions repeat="row view/repository_deletion_actions/delete">
+        <li>
+          <img src="/@@/no" title="Insufficient privileges"
+               tal:condition="not:row/allowed"/>
+              <tal:item tal:content="structure row/item/fmt:link" />
+              (<tal:reason tal:content="row/reason" />)
+        </li>
+      </tal:actions>
+    </ul>
+  </tal:deletelist>
+  <tal:alterlist condition="view/repository_deletion_actions/alter">
+    <div>The following items will be <em>updated</em>:</div>
+    <ul>
+      <tal:actions repeat="row view/repository_deletion_actions/alter">
+        <li>
+          <img src="/@@/no" title="Insufficient privileges"
+               tal:condition="not:row/allowed"/>
+              <tal:item tal:content="structure row/item/fmt:link" />
+              (<tal:reason tal:content="row/reason" />)
+        </li>
+      </tal:actions>
+    </ul>
+  </tal:alterlist>
+  <p tal:condition="view/all_permitted">
+      Repository deletion is permanent.
+  </p>
+  <p tal:condition="not:view/all_permitted">
+      You do not have permission to make all the changes required to delete
+      this repository.
+  </p>
+
+  <div metal:use-macro="context/@@launchpad_form/form" />
+
+</div>
+</body>
+</html>