Merge lp:~wgrant/launchpad/bug-592935-hide-disabled-ppas into lp:launchpad

[William Grant]

Hide non-Viewable PPAs from the person index. This prevents normal users from seeing others' disabled PPAs

Normal users are currently able to see the disabled and deleted PPAs of others on their index pages (eg. https://launchpad.net/~wgrant, the two or three stricken-out ones).

This is pointless, confusing, makes deletion look like a joke, and generally annoys users. As far as I can tell it is unintentional, as others I've talked to weren't aware that it was the case.

The fix is simple: just check for launchpad.View on the PPA, since that's revoked from non-owners when a PPA is disabled or deleted. This also has the side-effect of removing an empty <tr> which was in the place of invisible private PPAs.

[Māris Fogels (mars)]

Hi William,

A huge +1 from me for fixing this. You are right, the previous behaviour was bad. A nice, small, clean fix.

I'll assume you will run the full suite before landing this. You never know what other tests may unintentionally depend on the PPA link listing.

r=mars

Maris

review: Approve

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt	2010-05-09 21:11:52 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt	2010-06-12 05:20:44 +0000
@@ -644,33 +644,31 @@
     >>> admin_browser.getControl(name="field.enabled").value = False
     >>> admin_browser.getControl("Save").click()
 
-Anonymous access or users with no permission to view Celso's 'edge'
-ppa can see it listed in Celso's profile page.
+Anonymous users or others with no special permissions on the disabled PPA
+are unable to see it on Celso's profile page.
 
     >>> anon_browser.open("http://launchpad.dev/~cprov")
+    >>> print_tag_with_id(anon_browser.contents, 'ppas')
+    Personal package archives
+    PPA for Celso Providelo
+
     >>> browser.open("http://launchpad.dev/~cprov")
-
-    >>> print_tag_with_id(anon_browser.contents, 'ppas')
-    Personal package archives
-    Edge PPA
-    PPA for Celso Providelo
-
     >>> print_tag_with_id(browser.contents, 'ppas')
     Personal package archives
+    PPA for Celso Providelo
+
+Celso himself can see the PPA, and it's linked so he can reenable it if
+required.
+
+    >>> cprov_browser.open("http://launchpad.dev/~cprov")
+    >>> print_tag_with_id(cprov_browser.contents, 'ppas')
+    Personal package archives
     Edge PPA
     PPA for Celso Providelo
-
-However the reference to Celso's 'Edge PPA' is not linked.
-
-    >>> print anon_browser.getLink('Edge PPA')
-    Traceback (most recent call last):
-    ...
-    LinkNotFoundError
-
-    >>> print browser.getLink('Edge PPA')
-    Traceback (most recent call last):
-    ...
-    LinkNotFoundError
+    Create a new PPA
+
+    >>> print cprov_browser.getLink('Edge PPA')
+    <Link ...>
 
 And direct access to the PPA page is also denied.
 

=== modified file 'lib/lp/soyuz/templates/person-ppas.pt'
--- lib/lp/soyuz/templates/person-ppas.pt	2009-08-21 13:48:43 +0000
+++ lib/lp/soyuz/templates/person-ppas.pt	2010-06-12 05:20:44 +0000
@@ -6,10 +6,8 @@
   <div tal:define="ppas context/ppas" tal:condition="ppas">
     <table>
       <tal:ppa_line tal:repeat="ppa ppas">
-        <tr>
-          <td tal:define="ppa_link ppa/fmt:link"
-              tal:condition="ppa_link"
-              tal:content="structure ppa_link"></td>
+        <tr tal:condition="ppa/required:launchpad.View">
+          <td tal:content="structure ppa/fmt:link"></td>
         </tr>
       </tal:ppa_line>
     </table>