Code review comment for lp:~michael.nelson/launchpad/458200-p3a

Revision history for this message
Michael Nelson (michael.nelson) wrote :

= Summary =

This fixes bug 458200 where members viewing a team would not see the ppa
section if the team had a single private PPA.

== Proposed fix ==

Instead of showing the section when there is at least one public ppa, we
show the section when there is at least one ppa that the current user
can view.

== Pre-implementation notes ==

Muharem and I talked about this together during our pairing session.

== Implementation details ==

I'm a little uncertain about the tests - I couldn't see any other unit
tests for views that used a layer other than LaunchpadZopeless, but I
couldn't use LaunchpadZopeless as otherwise check_permission always
returns true. Is this OK - or is it reflecting that I'm doing something
I shouldn't be (like using check_permission in a view?).

== Tests ==

bin/test -vvt TestShouldShowPpaSection

== Demo and Q/A ==

To demo locally:

1. Login as <email address hidden>:test
2. visit https://launchpad.dev/~launchpad and create a new PPA called
ppaprivate.
3. Click on Administer archive and set the PPA to private (adding an
arbitrary buildd_secret)
4. re-visit https://launchpad.dev/~launchpad and verify the ppa section is
   there.
5. Click on active members, then add member, adding cprov.
6. logout, view https://launchpad.dev/~launchpad and confirm that PPA
section is not displayed.
7. login as <email address hidden>:cprov and confirm that the PPA
section *is* displayed.
8. logout and log back in as foo.bar and add a second ppa called publicppa.
9. logout and visit https://launchpad.dev/~launchpad and ensure that the
PPA section *is* displayed with just the public ppa listed.
10. login as cprov and both are listed.

If you do the same test on RF, point 7 will fail.

For QA we can ask the person on the bug to confirm that they now see the
ppa section.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/browser/tests/test_person_view.py
  lib/lp/registry/browser/person.py

== Pylint notices ==

lib/lp/registry/browser/person.py
    117: [F0401] Unable to import 'lazr.delegates' (No module named
delegates)
    118: [F0401] Unable to import 'lazr.config' (No module named config)
    119: [F0401] Unable to import 'lazr.restful.interface' (No module
named restful)
    230: [F0401] Unable to import 'lazr.uri' (No module named uri)

--
Michael

« Back to merge proposal