launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/more-trivialities into lp:launchpad.

Commit message:
Fix a few trivial UI bugs: Distribution:+index hides disabled feature links, blueprints on Milestone:+index have icons again and are sortable, BugTask:+addcomment's title doesn't duplicate the bug number, the Opinion bug status has a colour, and Branch:+index links more sensibly to merge proposals.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #80315 in Launchpad itself: "Don't make claims about builds for non-LP distributions"
  https://bugs.launchpad.net/launchpad/+bug/80315
  Bug #257627 in Launchpad itself: "Distributions that can't have PPAs still have "PPAs" pages"
  https://bugs.launchpad.net/launchpad/+bug/257627
  Bug #648645 in Launchpad itself: "Opinion shown in red color after transitioning from Incomplete"
  https://bugs.launchpad.net/launchpad/+bug/648645
  Bug #711647 in Launchpad itself: "branch page looks like it links to branches, actually links to merge proposals"
  https://bugs.launchpad.net/launchpad/+bug/711647
  Bug #1323808 in Launchpad itself: "Cut off bug number when adding an attachment"
  https://bugs.launchpad.net/launchpad/+bug/1323808
  Bug #1354387 in Launchpad itself: "Sorting by blueprint on milestone page not working"
  https://bugs.launchpad.net/launchpad/+bug/1354387

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/more-trivialities/+merge/270656

Fix a few trivial UI bugs: Distribution:+index hides disabled feature links, blueprints on Milestone:+index have icons again and are sortable, BugTask:+addcomment's title doesn't duplicate the bug number, the Opinion bug status has a colour, and Branch:+index links more sensibly to merge proposals.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/more-trivialities into lp:launchpad.

=== modified file 'lib/canonical/launchpad/icing/css/colours.css'
--- lib/canonical/launchpad/icing/css/colours.css	2012-10-26 04:42:32 +0000
+++ lib/canonical/launchpad/icing/css/colours.css	2015-09-10 11:14:06 +0000
@@ -106,7 +106,8 @@
     color: green;
     }
 .statusINVALID, .statusINVALID a,
-.statusWONTFIX, .statusWONTFIX a {
+.statusWONTFIX, .statusWONTFIX a,
+.statusOPINION, .statusOPINION a {
     color: #555;
     }
 .importanceCRITICAL, .importanceCRITICAL a {

=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2015-07-23 14:32:50 +0000
+++ lib/lp/app/browser/tales.py	2015-09-10 11:14:06 +0000
@@ -1047,6 +1047,9 @@
 
         return sprite_str
 
+    def icon(self):
+        return '<span class="%s"></span>' % self.sprite_css()
+
     def badges(self):
 
         badges = ''

=== modified file 'lib/lp/bugs/browser/bugmessage.py'
--- lib/lp/bugs/browser/bugmessage.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/browser/bugmessage.py	2015-09-10 11:14:06 +0000
@@ -28,15 +28,13 @@
     schema = IBugMessageAddForm
     initial_focus_widget = None
 
+    page_title = "Add a comment or attachment"
+
     @property
     def label(self):
         return 'Add a comment or attachment to bug #%d' % self.context.bug.id
 
     @property
-    def page_title(self):
-        return self.label
-
-    @property
     def initial_values(self):
         return dict(subject=self.context.bug.followup_subject())
 

=== modified file 'lib/lp/bugs/templates/bug-comment-add-form.pt'
--- lib/lp/bugs/templates/bug-comment-add-form.pt	2012-09-21 02:51:51 +0000
+++ lib/lp/bugs/templates/bug-comment-add-form.pt	2015-09-10 11:14:06 +0000
@@ -10,7 +10,7 @@
     >
       Remember, this bug report is a duplicate of
         <a href="#" tal:attributes="href context/bug/duplicateof/fmt:url">bug
-          #<span tal:replace="context/bug/duplicateof/id">42</span></a><br/>.
+          #<span tal:replace="context/bug/duplicateof/id">42</span></a>.<br/>
       Comment here only if you think the duplicate status is wrong.
     </div>
     <input

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2015-05-07 13:57:33 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2015-09-10 11:14:06 +0000
@@ -466,6 +466,7 @@
             # These values are extracted here and used below.
             linked_bug_rendered_text = "\n".join(linked_bug_text)
             mp_url = canonical_url(mp, force_local_path=True)
+            branch_url = canonical_url(mp.source_branch, force_local_path=True)
             branch_display_name = mp.source_branch.displayname
 
         browser = self.getUserBrowser(canonical_url(branch))
@@ -493,8 +494,9 @@
 
         links = revision_content.findAll('a')
         self.assertEqual(mp_url, links[2]['href'])
-        self.assertEqual(linked_bug_urls[0], links[3]['href'])
-        self.assertEqual(linked_bug_urls[1], links[4]['href'])
+        self.assertEqual(branch_url, links[3]['href'])
+        self.assertEqual(linked_bug_urls[0], links[4]['href'])
+        self.assertEqual(linked_bug_urls[1], links[5]['href'])
 
     def test_view_for_user_with_artifact_grant(self):
         # Users with an artifact grant for a branch related to a private

=== modified file 'lib/lp/code/templates/branch-macros.pt'
--- lib/lp/code/templates/branch-macros.pt	2015-06-10 10:25:28 +0000
+++ lib/lp/code/templates/branch-macros.pt	2015-09-10 11:14:06 +0000
@@ -274,8 +274,8 @@
     <div tal:define="merge_proposal python:rev_info['merge_proposal']"
          tal:condition="merge_proposal">
       <dd class="subordinate revision-comment">
-        Merged branch <a tal:attributes="href merge_proposal/fmt:url"
-                    tal:content="merge_proposal/source_branch/displayname">source branch</a>
+        <a tal:attributes="href merge_proposal/fmt:url">Merged</a> branch
+        <a tal:replace="structure merge_proposal/source_branch/fmt:link">source branch</a>
         <tal:linkedbugs
                 define="linked_bugtasks python:rev_info['linked_bugtasks']"
                 condition="linked_bugtasks">

=== modified file 'lib/lp/registry/browser/tests/test_distribution.py'
--- lib/lp/registry/browser/tests/test_distribution.py	2012-08-13 21:04:17 +0000
+++ lib/lp/registry/browser/tests/test_distribution.py	2015-09-10 11:14:06 +0000
@@ -9,6 +9,7 @@
 from lazr.restful.interfaces import IJSONRequestCache
 import soupmatchers
 from testtools.matchers import (
+    MatchesAll,
     MatchesAny,
     Not,
     )
@@ -19,6 +20,7 @@
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.webapp import canonical_url
 from lp.testing import (
+    admin_logged_in,
     login_celebrity,
     login_person,
     TestCaseWithFactory,
@@ -98,6 +100,37 @@
         self.assertThat(view.render(), series_header_match)
         self.assertThat(view.render(), Not(add_series_match))
 
+    def test_mirrors_links(self):
+        view = create_initialized_view(self.distro, "+index")
+        cd_mirrors_link = soupmatchers.HTMLContains(soupmatchers.Tag(
+            "CD mirrors link", "a", text="CD mirrors"))
+        archive_mirrors_link = soupmatchers.HTMLContains(soupmatchers.Tag(
+            "Archive mirrors link", "a", text="Archive mirrors"))
+        self.assertThat(
+            view(), Not(MatchesAny(cd_mirrors_link, archive_mirrors_link)))
+        with admin_logged_in():
+            self.distro.supports_mirrors = True
+        self.assertThat(
+            view(), MatchesAll(cd_mirrors_link, archive_mirrors_link))
+
+    def test_ppas_link(self):
+        view = create_initialized_view(self.distro, "+index")
+        ppas_link = soupmatchers.HTMLContains(soupmatchers.Tag(
+            "PPAs link", "a", text="Personal Package Archives"))
+        self.assertThat(view(), Not(ppas_link))
+        with admin_logged_in():
+            self.distro.supports_ppas = True
+        self.assertThat(view(), ppas_link)
+
+    def test_builds_link(self):
+        view = create_initialized_view(self.distro, "+index")
+        builds_link = soupmatchers.HTMLContains(soupmatchers.Tag(
+            "Builds link", "a", text="Builds"))
+        self.assertThat(view(), Not(builds_link))
+        with admin_logged_in():
+            self.distro.official_packages = True
+        self.assertThat(view(), builds_link)
+
 
 class TestDistributionView(TestCaseWithFactory):
     """Tests the DistributionView."""

=== modified file 'lib/lp/registry/templates/distribution-index.pt'
--- lib/lp/registry/templates/distribution-index.pt	2014-07-07 03:32:50 +0000
+++ lib/lp/registry/templates/distribution-index.pt	2015-09-10 11:14:06 +0000
@@ -97,16 +97,16 @@
 
       <div id="packages-archives" class="portlet">
         <ul>
-          <li>
+          <li tal:condition="context/supports_mirrors">
             <a tal:replace="structure overview_menu/cdimage_mirrors/fmt:link" />
           </li>
-          <li>
+          <li tal:condition="context/supports_mirrors">
             <a tal:replace="structure overview_menu/archive_mirrors/fmt:link" />
           </li>
-          <li>
+          <li tal:condition="context/supports_ppas">
             <a tal:replace="structure overview_menu/ppas/fmt:link" />
           </li>
-          <li>
+          <li tal:condition="context/official_packages">
             <a tal:replace="structure overview_menu/builds/fmt:link" />
           </li>
         </ul>

=== modified file 'lib/lp/registry/templates/milestone-index.pt'
--- lib/lp/registry/templates/milestone-index.pt	2015-01-29 18:48:37 +0000
+++ lib/lp/registry/templates/milestone-index.pt	2015-09-10 11:14:06 +0000
@@ -243,6 +243,7 @@
             <tbody>
               <tr tal:repeat="spec specs">
                 <td class="icon left">
+                  <span class="sortkey" tal:content="spec/title" />
                   <span tal:content="structure spec/image:icon" />
                 </td>
                 <td>