launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-repository-delete-access into lp:launchpad.

Commit message:
Fix deletion of Git repositories with access grants.

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-access/+merge/260009

Fix deletion of Git repositories with access grants.  I hope I got the access code right since I'm not as familiar with the model as I perhaps should be ...
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-repository-delete-access into lp:launchpad.

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2015-05-19 11:29:24 +0000
+++ lib/lp/code/model/gitrepository.py	2015-05-23 16:48:17 +0000
@@ -884,6 +884,10 @@
             operation()
         Store.of(self).flush()
 
+    def _deleteRepositoryAccessGrants(self):
+        """Delete access grants for this repository prior to deleting it."""
+        getUtility(IAccessArtifactSource).delete([self])
+
     def _deleteRepositorySubscriptions(self):
         """Delete subscriptions for this repository prior to deleting it."""
         subscriptions = Store.of(self).find(
@@ -918,6 +922,7 @@
                 "Cannot delete Git repository: %s" % self.unique_name)
 
         self.refs.remove()
+        self._deleteRepositoryAccessGrants()
         self._deleteRepositorySubscriptions()
         self._deleteJobs()
 

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2015-05-19 11:29:24 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2015-05-23 16:48:17 +0000
@@ -341,6 +341,26 @@
             self.user, BranchSubscriptionNotificationLevel.NOEMAIL, None,
             CodeReviewNotificationLevel.NOEMAIL, self.user)
         self.assertTrue(self.repository.canBeDeleted())
+        repository_id = self.repository.id
+        self.repository.destroySelf()
+        self.assertIsNone(
+            getUtility(IGitLookup).get(repository_id),
+            "The repository has not been deleted.")
+
+    def test_private_subscription_does_not_disable_deletion(self):
+        # A private repository that has a subscription can be deleted.
+        self.repository.transitionToInformationType(
+            InformationType.USERDATA, self.repository.owner,
+            verify_policy=False)
+        self.repository.subscribe(
+            self.user, BranchSubscriptionNotificationLevel.NOEMAIL, None,
+            CodeReviewNotificationLevel.NOEMAIL, self.user)
+        self.assertTrue(self.repository.canBeDeleted())
+        repository_id = self.repository.id
+        self.repository.destroySelf()
+        self.assertIsNone(
+            getUtility(IGitLookup).get(repository_id),
+            "The repository has not been deleted.")
 
     def test_landing_target_disables_deletion(self):
         # A repository with a landing target cannot be deleted.