launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:gitrepo-creating-warning into launchpad:master with ~pappacena/launchpad:gitrepo-status as a prerequisite.

Commit message:
Adding warning message and preventing user from deleting repositories directly from the interface while it is being created.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/386039
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:gitrepo-creating-warning into launchpad:master.

diff --git a/lib/lp/code/browser/gitrepository.py b/lib/lp/code/browser/gitrepository.py
index 4f38702..7dc1c32 100644
--- a/lib/lp/code/browser/gitrepository.py
+++ b/lib/lp/code/browser/gitrepository.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 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).
 
 """Git repository views."""
@@ -88,6 +88,7 @@ from lp.code.browser.widgets.gitrepositorytarget import (
     )
 from lp.code.enums import (
     GitGranteeType,
+    GitRepositoryStatus,
     GitRepositoryType,
     )
 from lp.code.errors import (
@@ -453,6 +454,12 @@ class GitRepositoryView(InformationTypePortletMixin, LaunchpadView,
             return False
         return scan_job.job.status == JobStatus.FAILED
 
+    @property
+    def warning_message(self):
+        if self.context.status == GitRepositoryStatus.CREATING:
+            return "This repository is being created."
+        return None
+
 
 class GitRepositoryRescanView(LaunchpadEditFormView):
 
@@ -1285,6 +1292,8 @@ class GitRepositoryDeletionView(LaunchpadFormView):
 
         Uses display_deletion_requirements as its source data.
         """
+        if self.context.status == GitRepositoryStatus.CREATING:
+            return False
         return len([item for item, action, reason, allowed in
             self.display_deletion_requirements if not allowed]) == 0
 
@@ -1325,6 +1334,12 @@ class GitRepositoryDeletionView(LaunchpadFormView):
     def cancel_url(self):
         return canonical_url(self.context)
 
+    @property
+    def warning_message(self):
+        if self.context.status == GitRepositoryStatus.CREATING:
+            return "This repository is being created and cannot be deleted."
+        return None
+
 
 class GitRepositoryActivityView(LaunchpadView):
 
diff --git a/lib/lp/code/browser/tests/test_gitrepository.py b/lib/lp/code/browser/tests/test_gitrepository.py
index a38120c..f59f53f 100644
--- a/lib/lp/code/browser/tests/test_gitrepository.py
+++ b/lib/lp/code/browser/tests/test_gitrepository.py
@@ -47,8 +47,8 @@ from lp.code.enums import (
     GitActivityType,
     GitGranteeType,
     GitPermissionType,
-    GitRepositoryType,
-    )
+    GitRepositoryType, GitRepositoryStatus,
+)
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.model.gitjob import GitRefScanJob
 from lp.code.tests.helpers import GitHostingFixture
@@ -138,6 +138,19 @@ class TestGitRepositoryView(BrowserTestCase):
             git clone git\+ssh://{username}@git.launchpad.test/.*
             """.format(username=username), text)
 
+    def test_creating_warning_message_is_present(self):
+        repository = removeSecurityProxy(self.factory.makeGitRepository())
+        repository.status = GitRepositoryStatus.CREATING
+        text = self.getMainText(repository, "+index", user=repository.owner)
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            r"""This repository is being created\..*""", text)
+
+    def test_creating_warning_message_is_not_shown(self):
+        repository = removeSecurityProxy(self.factory.makeGitRepository())
+        repository.status = GitRepositoryStatus.AVAILABLE
+        text = self.getMainText(repository, "+index", user=repository.owner)
+        self.assertNotIn("This repository is being created""", text)
+
     def test_user_can_push(self):
         # A user can push if they have edit permissions.
         repository = self.factory.makeGitRepository()
@@ -1802,6 +1815,42 @@ class TestGitRepositoryDeletionView(BrowserTestCase):
         delete_link = browser.getLink("Delete repository")
         self.assertEqual(delete_url, delete_link.url)
 
+    def test_creating_warning_message_is_present(self):
+        # If the repository is not being created, we should show the
+        # warning message, and suppress the "delete" button.
+        repository = removeSecurityProxy(self.factory.makeGitRepository())
+        repository.status = GitRepositoryStatus.CREATING
+        browser = self.getViewBrowser(
+            repository, "+delete", rootsite="code", user=repository.owner)
+
+        # Warning message is present.
+        tag = find_tags_by_class(browser.contents, "warning message", True)
+        self.assertIsNotNone(tag)
+        self.assertIn(
+            "This repository is being created and cannot be deleted.",
+            ' '.join(tag.contents))
+        # Delete button is not present
+        tag = find_tag_by_id(
+            browser.contents, "field.actions.delete_repository")
+        self.assertIsNone(tag)
+
+    def test_creating_warning_message_is_not_shown(self):
+        # If the repository is not being created, we should not show the
+        # warning message, and the "delete" button should be present.
+        repository = removeSecurityProxy(self.factory.makeGitRepository())
+        repository.status = GitRepositoryStatus.AVAILABLE
+        browser = self.getViewBrowser(
+            repository, "+delete", rootsite="code", user=repository.owner)
+
+        # Warning message is not present
+        tag = find_tags_by_class(browser.contents, "warning message", True)
+        self.assertIsNone(tag)
+        # Delete button is present
+        tag = find_tag_by_id(
+            browser.contents, "field.actions.delete_repository")
+        self.assertIsNotNone(tag)
+        self.assertEqual(tag.attrs['type'], 'submit')
+
     def test_warning_message(self):
         # The deletion view informs the user what will happen if they delete
         # the repository.
diff --git a/lib/lp/code/templates/gitrepository-delete.pt b/lib/lp/code/templates/gitrepository-delete.pt
index 2405139..3e92b6b 100644
--- a/lib/lp/code/templates/gitrepository-delete.pt
+++ b/lib/lp/code/templates/gitrepository-delete.pt
@@ -8,6 +8,13 @@
 <body>
 
   <div metal:fill-slot="main">
+  <div class="top-portlet" style="padding-top:0.5em;">
+    <p tal:condition="view/warning_message"
+       style="clear: right;" class="warning message"
+       tal:content="view/warning_message">
+      There is a warning here.
+    </p>
+  </div>
 
   <tal:deletelist condition="view/repository_deletion_actions/delete">
     The following items must be <em>deleted</em>:
diff --git a/lib/lp/code/templates/gitrepository-index.pt b/lib/lp/code/templates/gitrepository-index.pt
index c99665e..89f202f 100644
--- a/lib/lp/code/templates/gitrepository-index.pt
+++ b/lib/lp/code/templates/gitrepository-index.pt
@@ -32,6 +32,13 @@
 </tal:registering>
 
 <div metal:fill-slot="main">
+    <div class="top-portlet" style="padding-top:0.5em;">
+      <p tal:condition="view/warning_message"
+         style="clear: right;" class="warning message"
+         tal:content="view/warning_message">
+        There is a warning here.
+      </p>
+    </div>
 
   <div id="repository-description" tal:condition="context/description"
        class="summary"