launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/no-rescan-with-no-jobs into lp:launchpad with lp:~twom/launchpad/merge-proposal-rescan-link as a prerequisite.

Commit message:
No scan jobs existing is a valid state, don't show the rescan button in these situations

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/no-rescan-with-no-jobs/+merge/362479
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/no-rescan-with-no-jobs into lp:launchpad.

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2019-01-23 17:11:52 +0000
+++ lib/lp/code/browser/branch.py	2019-01-30 17:30:00 +0000
@@ -592,10 +592,10 @@
     def show_rescan_link(self):
         """Only show the rescan button if the latest scan has failed"""
         scan_job = self.context.getLatestScanJob()
-        # If there are no jobs, we failed to create one for some reason,
-        # so we should allow a rescan
+        # Having no jobs is a valid situation as there is a prune job.
+        # We don't need to allow a rescan
         if not scan_job:
-            return True
+            return False
         return scan_job.job.status == JobStatus.FAILED
 
     @cachedproperty

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2019-01-30 17:30:00 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2019-01-30 17:30:00 +0000
@@ -800,10 +800,13 @@
 
     @property
     def show_rescan_link(self):
-        latest_preview = self.context.getLatestScanJob()
-        if not latest_preview:
-            return True
-        return latest_preview.job.status == JobStatus.FAILED
+        """Only show the rescan button if the latest scan has failed"""
+        scan_job = self.context.getLatestScanJob()
+        # Having no jobs is a valid situation as there is a prune job.
+        # We don't need to allow a rescan
+        if not scan_job:
+            return False
+        return scan_job.job.status == JobStatus.FAILED
 
 
 @delegate_to(ICodeReviewVoteReference)

=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py	2019-01-28 15:36:56 +0000
+++ lib/lp/code/browser/gitrepository.py	2019-01-30 17:30:00 +0000
@@ -446,10 +446,10 @@
     def show_rescan_link(self):
         """Only show the rescan button if the latest scan has failed"""
         scan_job = self.context.getLatestScanJob()
-        # If there are no jobs, we failed to create one for some reason,
-        # so we should allow a rescan
+        # Having no jobs is a valid situation as there is a prune job.
+        # We don't need to allow a rescan
         if not scan_job:
-            return True
+            return False
         return scan_job.job.status == JobStatus.FAILED
 
 

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2019-01-23 18:14:05 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2019-01-30 17:30:00 +0000
@@ -322,7 +322,7 @@
         branch = self.factory.makeAnyBranch()
         view = create_initialized_view(branch, '+index')
         result = view.show_rescan_link
-        self.assertTrue(result)
+        self.assertFalse(result)
 
     def test_show_rescan_link_latest_didnt_fail(self):
         branch = self.factory.makeAnyBranch()
@@ -646,7 +646,7 @@
         logout()
         with StormStatementRecorder() as recorder:
             browser.open(branch_url)
-        self.assertThat(recorder, HasQueryCount(Equals(29)))
+        self.assertThat(recorder, HasQueryCount(Equals(30)))
 
 
 class TestBranchViewPrivateArtifacts(BrowserTestCase):

=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py	2019-01-28 17:19:44 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py	2019-01-30 17:30:00 +0000
@@ -445,7 +445,7 @@
         repository = self.factory.makeGitRepository()
         view = create_initialized_view(repository, '+index')
         result = view.show_rescan_link
-        self.assertTrue(result)
+        self.assertFalse(result)
 
     def test_show_rescan_link_latest_didnt_fail(self):
         repository = self.factory.makeGitRepository()