launchpad-reviewers team mailing list archive

[Ioana Lasc <mp+413748@xxxxxxxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:delete-revisionstatusreports-for-gitrepository into launchpad:master.

Commit message:
Delete status reports and artifacts in GitRepository.destroySelf

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/413748
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:delete-revisionstatusreports-for-gitrepository into launchpad:master.

diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
index d2d6a90..e901001 100644
--- a/lib/lp/code/interfaces/gitrepository.py
+++ b/lib/lp/code/interfaces/gitrepository.py
@@ -942,6 +942,9 @@ class IRevisionStatusReportSet(Interface):
     def findByCommit(repository, commit_sha1):
         """Returns all `RevisionStatusReport` for a repository and commit."""
 
+    def deleteForRepository(repository):
+        """Delete all `RevisionStatusReport` for a repository."""
+
 
 class IRevisionStatusArtifactSet(Interface):
     """The set of all revision status artifacts."""
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index 6d4fd11..f8ddbc9 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -394,6 +394,14 @@ class RevisionStatusReportSet:
                 RevisionStatusReport.date_created,
                 RevisionStatusReport.id)
 
+    def deleteForRepository(self, repository):
+        reports = self.findByRepository(repository)
+        for report in reports:
+            IStore(RevisionStatusArtifact).find(
+                RevisionStatusArtifact,
+                RevisionStatusArtifact.report == report).remove()
+        reports.remove()
+
 
 class RevisionStatusArtifact(StormBase):
     __storm_table__ = 'RevisionStatusArtifact'
@@ -1944,6 +1952,7 @@ class GitRepository(StormBase, WebhookTargetMixin, AccessTokenTargetMixin,
         # activity logs for removed repositories anyway.
         self.grants.remove()
         self.rules.remove()
+        getUtility(IRevisionStatusReportSet).deleteForRepository(self)
 
         # Now destroy the repository.
         repository_name = self.unique_name
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 6155836..61496c3 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -818,6 +818,45 @@ class TestGitRepositoryDeletion(TestCaseWithFactory):
             getUtility(IGitLookup).get(repository_id),
             "The repository has not been deleted.")
 
+    def test_revisionstatureports_do_not_disable_deletion(self):
+        title = self.factory.getUniqueUnicode('report-title')
+        result_summary = "120/120 tests passed"
+        commit_sha1 = hashlib.sha1(
+            self.factory.getUniqueBytes()).hexdigest()
+        result_summary2 = "Lint"
+        title2 = "Invalid import in test_file.py"
+
+        self.factory.makeRevisionStatusReport(
+            user=self.repository.owner, git_repository=self.repository,
+            title=title, commit_sha1=commit_sha1,
+            result_summary=result_summary,
+            result=RevisionStatusResult.SUCCEEDED)
+
+        report2 = self.factory.makeRevisionStatusReport(
+            user=self.repository.owner, git_repository=self.repository,
+            title=title2, commit_sha1=commit_sha1,
+            result_summary=result_summary2,
+            result=RevisionStatusResult.FAILED)
+
+        self.factory.makeRevisionStatusArtifact(report=report2)
+        self.factory.makeRevisionStatusArtifact(report=report2)
+        self.assertEqual(2, len(list(
+            getUtility(IRevisionStatusArtifactSet).findByReport(report2))))
+
+        self.assertTrue(
+            self.repository.canBeDeleted(),
+            "A newly created repository should be able to be deleted.")
+        repository_id = self.repository.id
+        self.repository.destroySelf()
+        self.assertIsNone(
+            getUtility(IGitLookup).get(repository_id),
+            "The repository has not been deleted.")
+        results = list(getUtility(IRevisionStatusReportSet).findByRepository(
+            self.repository))
+        self.assertEqual(0, len(results))
+        self.assertEqual(0, len(list(
+            getUtility(IRevisionStatusArtifactSet).findByReport(report2))))
+
     def test_subscription_does_not_disable_deletion(self):
         # A repository that has a subscription can be deleted.
         self.repository.subscribe(