launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/merge-proposal-rescan-link into lp:launchpad.

Commit message:
Add rescan button to branch merge proposals.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2018-09-06 16:00:45 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2019-01-30 16:16:13 +0000
@@ -57,6 +57,7 @@
     SimpleTerm,
     SimpleVocabulary,
     )
+from zope.security.proxy import removeSecurityProxy
 
 from lp import _
 from lp.app.browser.launchpadform import (
@@ -101,6 +102,7 @@
     )
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
+from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.interfaces.client import LibrarianServerError
 from lp.services.propertycache import (
     cachedproperty,
@@ -797,6 +799,13 @@
                 'launchpad.Edit', self.context),
             })
 
+    @property
+    def show_rescan_link(self):
+        latest_preview = self.context.getLatestScanJob()
+        if not latest_preview:
+            return True
+        return latest_preview.job.status == JobStatus.FAILED
+
 
 @delegate_to(ICodeReviewVoteReference)
 class DecoratedCodeReviewVoteReference:
@@ -922,6 +931,19 @@
                       reverse=True)
 
 
+class BranchMergeProposalRescanView(LaunchpadEditFormView):
+
+    schema = Interface
+
+    field_names = []
+
+    @action('Rescan', name='rescan')
+    def rescan(self, action, data):
+        self.context.rescan()
+        self.request.response.addNotification("Diff scan scheduled")
+        self.next_url = canonical_url(self.context)
+
+
 class IReviewRequest(Interface):
     """Schema for requesting a review."""
 

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2019-01-28 17:34:01 +0000
+++ lib/lp/code/browser/configure.zcml	2019-01-30 16:16:13 +0000
@@ -259,6 +259,12 @@
         permission="launchpad.Edit"
         template="../../app/templates/generic-edit.pt"/>
     <browser:page
+        name="+rescan"
+        for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
+        class="lp.code.browser.branchmergeproposal.BranchMergeProposalRescanView"
+        permission="launchpad.Edit"
+        template="../templates/branchmergeproposal-rescan.pt"/>
+    <browser:page
         name="+linkbug"
         for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
         class="lp.bugs.browser.buglinktarget.BugLinkView"

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2018-09-27 13:50:06 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2019-01-30 16:16:13 +0000
@@ -74,6 +74,7 @@
     IMergeProposalNeedsReviewEmailJobSource,
     IMergeProposalUpdatedEmailJobSource,
     )
+from lp.code.model.branchmergeproposaljob import UpdatePreviewDiffJob
 from lp.code.model.diff import PreviewDiff
 from lp.code.tests.helpers import (
     add_revision_to_branch,
@@ -89,7 +90,9 @@
     PersonVisibility,
     TeamMembershipPolicy,
     )
+from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
+from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.interfaces.client import LibrarianServerError
 from lp.services.messages.model.message import MessageSet
 from lp.services.timeout import TimeoutError
@@ -2147,6 +2150,33 @@
         browser = self.getViewBrowser(comment.branch_merge_proposal)
         self.assertIn('x y' * 100, browser.contents)
 
+    def test_show_rescan_link(self):
+        bmp = self.factory.makeBranchMergeProposal()
+        job = removeSecurityProxy(bmp.getLatestScanJob())
+        job.job._status = JobStatus.FAILED
+        view = create_initialized_view(bmp, '+index')
+        result = view.show_rescan_link
+        self.assertTrue(result)
+
+    def test_show_rescan_link_no_failures(self):
+        bmp = self.factory.makeBranchMergeProposal()
+        job = removeSecurityProxy(bmp.getLatestScanJob())
+        job.job._status = JobStatus.COMPLETED
+        job.job.date_finished = UTC_NOW
+        view = create_initialized_view(bmp, '+index')
+        result = view.show_rescan_link
+        self.assertFalse(result)
+
+    def test_show_rescan_link_latest_didnt_fail(self):
+        bmp = self.factory.makeBranchMergeProposal()
+        job = removeSecurityProxy(bmp.getLatestScanJob())
+        job.job._status = JobStatus.FAILED
+        job = UpdatePreviewDiffJob.create(bmp)
+        job.job._status = JobStatus.COMPLETED
+        view = create_initialized_view(bmp, '+index')
+        result = view.show_rescan_link
+        self.assertTrue(result)
+
 
 class TestLatestProposalsForEachBranchMixin:
     """Confirm that the latest branch is returned."""

=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py	2018-10-21 17:32:46 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py	2019-01-30 16:16:13 +0000
@@ -516,6 +516,11 @@
         :param previewdiff_id: The ID of the target `PreviewDiff`.
         """
 
+    def getLatestScanJob():
+        """Return the latest IGenerateIncrementalDiffJob and
+        IUpdatePreviewDiffJob for this repository.
+        """
+
 
 class IBranchMergeProposalEdit(Interface):
 
@@ -662,6 +667,14 @@
             the current description).
         """
 
+    @export_write_operation()
+    @operation_for_version("devel")
+    def rescan():
+        """Force a rescan of the diff for this branch merge proposal.
+
+        Thus may be helpful in cases where a previous scan has crashed.
+        """
+
     @operation_parameters(
         reviewer=Reference(
             title=_("A reviewer."), schema=IPerson),

=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py	2018-09-19 12:53:51 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2019-01-30 16:16:13 +0000
@@ -1220,6 +1220,26 @@
         if return_jobs:
             return jobs
 
+    def rescan(self):
+        """See `IBranchMergeProposal`."""
+        self.scheduleDiffUpdates()
+
+    def getLatestScanJob(self):
+        """See `IBranchMergeProposal`."""
+        from lp.code.model.branchmergeproposaljob import (
+            BranchMergeProposalJob,
+            UpdatePreviewDiffJob,
+            )
+        diff_type = UpdatePreviewDiffJob.class_job_type
+        latest_preview = IStore(BranchMergeProposalJob).find(
+            BranchMergeProposalJob,
+            BranchMergeProposalJob.branch_merge_proposal == self,
+            BranchMergeProposalJob.job_type == diff_type,
+            ).order_by(Desc(Job.date_finished)).first()
+        if latest_preview is not None:
+            latest_preview = latest_preview.makeDerived()
+        return latest_preview
+
     @property
     def revision_end_date(self):
         """The cutoff date for showing revisions.

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2018-10-21 17:32:46 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2019-01-30 16:16:13 +0000
@@ -2222,6 +2222,41 @@
         [job] = self.job_source.iterReady()
         self.assertIsInstance(job, UpdatePreviewDiffJob)
 
+    def test_getLatestScanJob(self):
+        complete_date = datetime.now(UTC)
+
+        bmp = self.factory.makeBranchMergeProposal()
+        failed_job = removeSecurityProxy(bmp.getLatestScanJob())
+        failed_job.job._status = JobStatus.FAILED
+        failed_job.job.date_finished = complete_date
+        completed_job = UpdatePreviewDiffJob.create(bmp)
+        completed_job.job._status = JobStatus.COMPLETED
+        completed_job.job.date_finished = complete_date - timedelta(seconds=10)
+        result = removeSecurityProxy(bmp.getLatestScanJob())
+        self.assertEqual(failed_job.job_id, result.job_id)
+
+    def test_getLatestScanJob_correct_branch(self):
+        complete_date = datetime.now(UTC)
+
+        main_bmp = self.factory.makeBranchMergeProposal()
+        second_bmp = self.factory.makeBranchMergeProposal()
+        failed_job = removeSecurityProxy(second_bmp.getLatestScanJob())
+        failed_job.job._status = JobStatus.FAILED
+        failed_job.job.date_finished = complete_date
+        completed_job = removeSecurityProxy(main_bmp.getLatestScanJob())
+        completed_job.job._status = JobStatus.COMPLETED
+        completed_job.job.date_finished = complete_date - timedelta(seconds=10)
+        result = removeSecurityProxy(main_bmp.getLatestScanJob())
+        self.assertEqual(completed_job.job_id, result.job_id)
+
+    def test_getLatestScanJob_without_completion_date(self):
+        bmp = self.factory.makeBranchMergeProposal()
+        failed_job = removeSecurityProxy(bmp.getLatestScanJob())
+        failed_job.job._status = JobStatus.FAILED
+        result = bmp.getLatestScanJob()
+        self.assertTrue(result)
+        self.assertIsNone(result.job.date_finished)
+
 
 class TestNextPreviewDiffJob(TestCaseWithFactory):
 

=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt	2018-04-25 12:01:33 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt	2019-01-30 16:16:13 +0000
@@ -182,7 +182,7 @@
   </div>
 
   <div id="diff-area">
-    <div class="yui-g" tal:condition="view/pending_diff">
+    <div class="yui-g" tal:condition="python: not view.show_rescan_link and view.pending_diff">
       <div class="pending-update" id="diff-pending-update">
         <h3>Updating diff...</h3>
         <p tal:condition="not: features/longpoll.merge_proposals.enabled">
@@ -194,6 +194,20 @@
         </p>
       </div>
     </div>
+    <div class="yui-g" tal:condition="view/show_rescan_link">
+      <div class="pending-update" id="diff-pending-update">
+        <h3>Diff calculation failed</h3>
+        <p>
+          Calculating the branch diff failed. You can manually rescan if required.
+        </p>
+        <p>
+          <form action="+rescan" name="launchpadform" method="post" enctype="multipart/form-data" accept-charset="UTF-8">
+            <input id="field.actions.rescan" class="button" type="submit"
+             name="field.actions.rescan" value="Rescan" />
+          </form>
+        </p>
+      </div>
+    </div>
     <div id="review-diff" tal:condition="view/preview_diff">
       <h2>Preview Diff </h2>
 

=== added file 'lib/lp/code/templates/branchmergeproposal-rescan.pt'
--- lib/lp/code/templates/branchmergeproposal-rescan.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/branchmergeproposal-rescan.pt	2019-01-30 16:16:13 +0000
@@ -0,0 +1,14 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml";
+  xmlns:tal="http://xml.zope.org/namespaces/tal";
+  xmlns:metal="http://xml.zope.org/namespaces/metal";
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+  metal:use-macro="view/macro:page/main_only"
+  i18n:domain="launchpad">
+  <body>
+    <div metal:fill-slot="main">
+      <p>You can schedule a rescan for this merge proposal if it appears the diff is out of date.</p>
+      <div metal:use-macro="context/@@launchpad_form/form" />
+    </div>
+  </body>
+</html>