launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-fix-mp-commits into lp:launchpad.

Commit message:
Fix rendering of merge proposals with deleted source Git refs.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-fix-mp-commits/+merge/295135

Even after I fixed a similar problem in https://code.launchpad.net/~cjwatson/launchpad/git-commits-link-mps/+merge/294653 following code review, I managed to miss a case in git-mp-commits where merge proposals would fail to render if their source ref has been deleted.  I caught it in QA, and this fixes that up.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-fix-mp-commits into lp:launchpad.

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2016-05-18 22:31:42 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2016-05-19 01:26:50 +0000
@@ -98,6 +98,7 @@
 from lp.testing.pages import (
     extract_text,
     find_tag_by_id,
+    first_tag_by_class,
     get_feedback_messages,
     )
 from lp.testing.views import create_initialized_view
@@ -1560,6 +1561,33 @@
                 content=description[:497] + '...'))
         self.assertThat(browser.contents, HTMLContains(expected_meta))
 
+    def test_unmerged_commits_from_deleted_git_ref(self):
+        # Even if the source Git ref has been deleted, we still know its tip
+        # SHA-1 and can ask the repository for its unmerged commits.
+        bmp = self.factory.makeBranchMergeProposalForGit()
+        sha1 = unicode(hashlib.sha1(b'0').hexdigest())
+        epoch = datetime.fromtimestamp(0, tz=pytz.UTC)
+        commit_date = datetime(2015, 1, 1, tzinfo=pytz.UTC)
+        hosting_client = FakeMethod()
+        hosting_client.getLog = FakeMethod(result=[
+            {
+                u'sha1': sha1,
+                u'message': u'Sample message',
+                u'author': {
+                    u'name': 'Example Person',
+                    u'email': 'person@xxxxxxxxxxx',
+                    u'time': int((commit_date - epoch).total_seconds()),
+                    },
+                }
+            ])
+        bmp.source_git_repository.removeRefs([bmp.source_git_path])
+        self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+        browser = self.getUserBrowser(canonical_url(bmp, rootsite='code'))
+        tag = first_tag_by_class(browser.contents, 'commit-details')
+        self.assertEqual(
+            "%.7s...\nby\nExample Person <person@xxxxxxxxxxx>\n"
+            "on 2015-01-01" % sha1, extract_text(tag))
+
 
 class TestBranchMergeProposalBrowserView(
     GitHostingClientMixin, BrowserTestCase):

=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt	2016-05-13 17:16:40 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt	2016-05-19 01:26:50 +0000
@@ -171,9 +171,9 @@
         </tal:remote-branch>
       </tal:bzr-revisions>
       <tal:git-revisions condition="context/source_git_ref">
-        <tal:history-available condition="context/source_git_ref/has_commits"
-                               define="ref context/source_git_ref;
-                                       commit_infos view/unlanded_revisions">
+        <tal:history-available define="ref context/source_git_ref;
+                                       commit_infos view/unlanded_revisions"
+                               condition="commit_infos">
           <h2>Unmerged commits</h2>
           <metal:commits use-macro="ref/@@+macros/ref-commits"/>
         </tal:history-available>

=== modified file 'lib/lp/code/templates/branchmergeproposal-resubmit.pt'
--- lib/lp/code/templates/branchmergeproposal-resubmit.pt	2016-05-13 17:16:40 +0000
+++ lib/lp/code/templates/branchmergeproposal-resubmit.pt	2016-05-19 01:26:50 +0000
@@ -39,9 +39,9 @@
       </tal:remote-branch>
     </tal:bzr-revisions>
     <tal:git-revisions condition="context/source_git_ref">
-      <tal:history-available condition="context/source_git_ref/has_commits"
-                             define="ref context/source_git_ref;
-                                     commit_infos view/unlanded_revisions">
+      <tal:history-available define="ref context/source_git_ref;
+                                     commit_infos view/unlanded_revisions"
+                             condition="commit_infos">
         <h2>Unmerged commits</h2>
         <metal:commits use-macro="ref/@@+macros/ref-commits"/>
       </tal:history-available>