launchpad-reviewers team mailing list archive

[Colin Watson <mp+416820@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:delete-related-access-tokens into launchpad:master.

Commit message:
Delete related access tokens when deleting a Git repository

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1964800 in Launchpad itself: "Can't delete git repository with access tokens"
  https://bugs.launchpad.net/launchpad/+bug/1964800

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/416820
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:delete-related-access-tokens into launchpad:master.

diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index 0dd205b..c52fd5e 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -1821,6 +1821,7 @@ class GitRepository(StormBase, WebhookTargetMixin, AccessTokenTargetMixin,
         # activity logs for removed repositories anyway.
         self.grants.remove()
         self.rules.remove()
+        removeSecurityProxy(self.getAccessTokens()).remove()
         getUtility(IRevisionStatusReportSet).deleteForRepository(self)
         getUtility(ICIBuildSet).deleteByGitRepository(self)
 
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index fd1f9eb..f7f19a6 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -1184,6 +1184,17 @@ class TestGitRepositoryDeletion(TestCaseWithFactory):
             GitActivity, GitActivity.repository_id == repository_id)
         self.assertEqual([], list(activities))
 
+    def test_related_access_tokens_deleted(self):
+        _, token = self.factory.makeAccessToken(target=self.repository)
+        other_repository = self.factory.makeGitRepository()
+        _, other_token = self.factory.makeAccessToken(target=other_repository)
+        self.repository.destroySelf()
+        transaction.commit()
+        self.assertRaises(
+            LostObjectError, getattr, removeSecurityProxy(token), 'target')
+        self.assertEqual(
+            other_repository, removeSecurityProxy(other_token).target)
+
     def test_related_ci_builds_deleted(self):
         # A repository that has a CI build can be deleted.
         build = self.factory.makeCIBuild(git_repository=self.repository)