launchpad-reviewers team mailing list archive

[Maximiliano Bertacchini <maximiliano.bertacchini@xxxxxxxxxxxxx>]

Nice feature, thanks! Just a quick review, with a couple of questions.

Diff comments:

> 
> === modified file 'lib/lp/code/model/branch.py'
> --- lib/lp/code/model/branch.py	2018-12-10 13:54:34 +0000
> +++ lib/lp/code/model/branch.py	2019-01-23 16:23:25 +0000
> @@ -1295,6 +1295,15 @@
>              job.celeryRunOnCommit()
>          return (self.last_mirrored_id, old_scanned_id)
>  
> +    def getLatestScanJob(self):
> +        from lp.code.model.branchjob import BranchJob, BranchScanJob
> +        latest_job = IStore(BranchJob).find(
> +            BranchJob,
> +            branch=self,
> +            job_type=BranchScanJob.class_job_type).order_by(

Should the ORDER BY be DESC?

> +                BranchJob.id).first()
> +        return latest_job
> +
>      def requestMirror(self):
>          """See `IBranch`."""
>          if self.branch_type in (BranchType.REMOTE, BranchType.HOSTED):
> 
> === modified file 'lib/lp/code/model/tests/test_branch.py'
> --- lib/lp/code/model/tests/test_branch.py	2018-07-09 09:27:06 +0000
> +++ lib/lp/code/model/tests/test_branch.py	2019-01-23 16:23:25 +0000
> @@ -3502,6 +3502,19 @@
>                  getUtility(ILaunchpadCelebrities).commercial_admin):
>              branch.unscan()
>  
> +    def test_getLatestScanJob(self):
> +        branch = self.factory.makeAnyBranch()
> +        with person_logged_in(branch.owner):
> +            branch.unscan(rescan=True)

Maybe make sure there are 2+ ScanJobs and check the returned one is actually the latest?

> +        found = Store.of(branch).find(BranchJob, branch=branch)[0]
> +        result = branch.getLatestScanJob()
> +        self.assertEqual(found, result)
> +
> +    def test_getLatestScanJob_no_scans(self):
> +        branch = self.factory.makeAnyBranch()
> +        result = branch.getLatestScanJob()
> +        self.assertIsNone(result)
> +
>  
>  class TestWebservice(TestCaseWithFactory):
>      """Tests for the webservice."""
> 
> === modified file 'lib/lp/code/templates/branch-index.pt'
> --- lib/lp/code/templates/branch-index.pt	2017-11-06 09:32:45 +0000
> +++ lib/lp/code/templates/branch-index.pt	2019-01-23 16:23:25 +0000
> @@ -130,18 +130,38 @@
>  
>    </div>
>  
> -  <div class="yui-g" tal:condition="view/pending_updates">
> +  <tal:rescan condition="not:view/show_rescan_link">
> +    <div class="yui-g" tal:condition="view/pending_updates">
> +      <div class="portlet">
> +        <div id="branch-pending-updates" class="pending-update">
> +          <h3>Updating branch...</h3>
> +          <p>
> +            Launchpad is processing new changes to this branch which will be
> +            available in a few minutes. Reload to see the changes.
> +          </p>
> +        </div>
> +      </div>
> +    </div>
> +  </tal:rescan>
> +
> +  <div class="yui-g" tal:condition="view/show_rescan_link">
>      <div class="portlet">
> -      <div id="branch-pending-updates" class="pending-update">
> -        <h3>Updating branch...</h3>
> -        <p>
> -          Launchpad is processing new changes to this branch which will be
> -          available in a few minutes. Reload to see the changes.
> +      <div id="branch-scan-failed" class="pending-update">
> +        <h3>Branch scan failed</h3>
> +        <p>
> +          Scanning this branch for changes has failed, you can manually rescan if required.

I'd replace the ',' with '.' or ':'. Also, maybe "... you can request a rescan ..."?

> +        </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>
>  
> +
>    <div class="yui-g">
>      <div class="portlet" id="recent-revisions">
>        <h2>Recent revisions</h2>


-- 
https://code.launchpad.net/~twom/launchpad/manually-rescan-link/+merge/362139
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.