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

Proposed by William Grant
Status: Merged
Approved by: Māris Fogels
Approved revision: no longer in the source branch.
Merged at revision: 11017
Proposed branch: lp:~wgrant/launchpad/bug-592935-hide-disabled-ppas
Merge into: lp:launchpad
Diff against target: 72 lines (+20/-24)
2 files modified
lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt (+18/-20)
lib/lp/soyuz/templates/ (+2/-4)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-592935-hide-disabled-ppas
Reviewer Review Type Date Requested Status
Māris Fogels (community) Approve
Review via email:

Commit message

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

Description of the change

Normal users are currently able to see the disabled and deleted PPAs of others on their index pages (eg., 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.

To post a comment you must log in.
Revision history for this message
Māris Fogels (mars) wrote :

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.



review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt'
2--- lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt 2010-05-09 21:11:52 +0000
3+++ lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt 2010-06-12 05:20:44 +0000
4@@ -644,33 +644,31 @@
5 >>> admin_browser.getControl(name="field.enabled").value = False
6 >>> admin_browser.getControl("Save").click()
8-Anonymous access or users with no permission to view Celso's 'edge'
9-ppa can see it listed in Celso's profile page.
10+Anonymous users or others with no special permissions on the disabled PPA
11+are unable to see it on Celso's profile page.
13 >>>"")
14+ >>> print_tag_with_id(anon_browser.contents, 'ppas')
15+ Personal package archives
16+ PPA for Celso Providelo
18 >>>"")
20- >>> print_tag_with_id(anon_browser.contents, 'ppas')
21- Personal package archives
22- Edge PPA
23- PPA for Celso Providelo
25 >>> print_tag_with_id(browser.contents, 'ppas')
26 Personal package archives
27+ PPA for Celso Providelo
29+Celso himself can see the PPA, and it's linked so he can reenable it if
32+ >>>"")
33+ >>> print_tag_with_id(cprov_browser.contents, 'ppas')
34+ Personal package archives
35 Edge PPA
36 PPA for Celso Providelo
38-However the reference to Celso's 'Edge PPA' is not linked.
40- >>> print anon_browser.getLink('Edge PPA')
41- Traceback (most recent call last):
42- ...
43- LinkNotFoundError
45- >>> print browser.getLink('Edge PPA')
46- Traceback (most recent call last):
47- ...
48- LinkNotFoundError
49+ Create a new PPA
51+ >>> print cprov_browser.getLink('Edge PPA')
52+ <Link ...>
54 And direct access to the PPA page is also denied.
57=== modified file 'lib/lp/soyuz/templates/'
58--- lib/lp/soyuz/templates/ 2009-08-21 13:48:43 +0000
59+++ lib/lp/soyuz/templates/ 2010-06-12 05:20:44 +0000
60@@ -6,10 +6,8 @@
61 <div tal:define="ppas context/ppas" tal:condition="ppas">
62 <table>
63 <tal:ppa_line tal:repeat="ppa ppas">
64- <tr>
65- <td tal:define="ppa_link ppa/fmt:link"
66- tal:condition="ppa_link"
67- tal:content="structure ppa_link"></td>
68+ <tr tal:condition="ppa/required:launchpad.View">
69+ <td tal:content="structure ppa/fmt:link"></td>
70 </tr>
71 </tal:ppa_line>
72 </table>