launchpad-reviewers team mailing list archive

[Ioana Lasc <mp+413405@xxxxxxxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:show-expander-only-when-status-reports-exist into launchpad:master.

Commit message:
Display expander only when status reports exist for commit

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/413405
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:show-expander-only-when-status-reports-exist into launchpad:master.

diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
index 8546c4c..979927c 100644
--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
@@ -1678,6 +1678,12 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
                     "first report summary", "td",
                     text=report1.result_summary))
 
+            # Ensure we don't display an empty expander for those commits
+            # that do not have status reports created for them - means we
+            # should only see one entry with class 'status-reports-table'
+            # on the page: reports_section[0]
+            self.assertTrue(len(reports_section) == 1)
+
     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.
diff --git a/lib/lp/code/browser/tests/test_gitref.py b/lib/lp/code/browser/tests/test_gitref.py
index 96a9a8d..51178fa 100644
--- a/lib/lp/code/browser/tests/test_gitref.py
+++ b/lib/lp/code/browser/tests/test_gitref.py
@@ -225,6 +225,12 @@ class TestGitRefView(BrowserTestCase):
                     "second report summary", "td",
                     text=report2.result_summary))
 
+            # Ensure we don't display an empty expander for those commits
+            # that do not have status reports created for them - means we
+            # should only see 2 entries with class 'status-reports-table'
+            # on the page: reports_section[0] and reports_section[1]
+            self.assertTrue(len(reports_section) == 2)
+
     def test_clone_instructions(self):
         [ref] = self.factory.makeGitRefs(paths=["refs/heads/branch"])
         username = ref.owner.name
diff --git a/lib/lp/code/templates/git-macros.pt b/lib/lp/code/templates/git-macros.pt
index ea2ecf1..836409d 100644
--- a/lib/lp/code/templates/git-macros.pt
+++ b/lib/lp/code/templates/git-macros.pt
@@ -217,30 +217,28 @@
       replace="structure commit_message/fmt:obfuscate-email/fmt:text-to-html" />
   </dd>
   <dd class="subordinate collapsible status-reports-table"
-      tal:condition="show_status_reports | nothing"
       tal:define="
         commit_sha1 commit_info/sha1;
-        status_result python:ref.getCommitStatus(commit_sha1)">
+        status_result python:ref.getCommitStatus(commit_sha1);
+        batchnav python:view.getStatusReports(commit_sha1) if show_status_reports else None;
+        batch python:batchnav.currentBatch() if show_status_reports else None"
+      tal:condition="python: show_status_reports and batch">
       <div class="sprite treeCollapsed">
         <span tal:replace="structure status_result/image:boolean" />
       </div>
-      <tal:batch
-        define="batchnav python:view.getStatusReports(commit_sha1);
-                batch batchnav/currentBatch">
-        <table class="listing" tal:condition="batch">
-          <tbody>
-            <tr tal:repeat="report batch">
-              <td tal:content="structure report/result/enumvalue:SUCCEEDED/image:boolean" />
-              <td><a tal:attributes="href report/url|nothing" tal:content="report/title" /></td>
-              <td tal:content="report/result_summary" />
-            </tr>
-          </tbody>
-        </table>
-        <div class="lower-batch-nav">
-          <tal:navigation
-            replace="structure batchnav/@@+navigation-links-lower" />
-        </div>
-      </tal:batch>
+      <table class="listing" tal:condition="batch">
+        <tbody>
+          <tr tal:repeat="report batch">
+            <td tal:content="structure report/result/enumvalue:SUCCEEDED/image:boolean" />
+            <td><a tal:attributes="href report/url|nothing" tal:content="report/title" /></td>
+            <td tal:content="report/result_summary" />
+          </tr>
+        </tbody>
+      </table>
+      <div class="lower-batch-nav">
+        <tal:navigation
+          replace="structure batchnav/@@+navigation-links-lower" />
+      </div>
   </dd>
   <div tal:define="merge_proposal python:commit_info.get('merge_proposal')"
        tal:condition="merge_proposal">