launchpad-reviewers team mailing list archive

[Henning Eggers <henning.eggers@xxxxxxxxxxxxx>]

The proposal to merge lp:~henninge/launchpad/devel-bug-638920-private-branch into lp:launchpad/devel has been updated.

Description changed to:

= Bug 638920 =

The translations page for a product series shows links the import and export Bazaar branches, if there are any. If either of these branches is private and the user no permissions to view them, the page fails with "unauthorized". The main page for a product series also displays a link to the series branch (which is the import branch in case of translations) but if it is private, it pretends that no branch has been set at all. Yes, that's flat out lying ;-). But since that page gets away with, the translations page can (in fact must) do the same.

== Proposed fix ==

Change the has_imports_enabled and has_exports_enabled flags on the view class to check for "View" permission on the branch. If the user lacks permission, the should return "False" just like when no imports or exports have been enabled.

== Implementation details ==

The actual fix is a few lines in lp/translations/browser/productseries.py that add a check_permission call to each of the two properties. It also removes a redundant check from uses_bzr_sync which is now the simple "or" combination of the other two flags.

The rest of the branch consists of tests for the two flags which were missing before and a pagetest that reproduced the bug nicely.

== Tests ==

I learned a little more about test layers because the new test would not work on the ZopelessLayer but needs the FunctionalLayer instead to be able to check permissions. Notice how I removed the security proxy from the productseries to be able to set it up (assign a branch, set import mode) for the test but the view itself is created using the secured copy of the productseries.
LaunchpadZopelessLayer which includes the LibrarianLayer and MemcachedLayer is not needed, though, so I changed that for the other tests.

bin/test -vvcm lp.translations \
    -t TestProductSeriesViewBzrUsage \
    -t xx-productseries-translations.txt

== Demo and QA ==

See my little screen cast that I produced for this just for the fun of it. ;-)

http://people.canonical.com/~henninge/screencasts/private_branch_link.ogv

(Please forgive my blowing into the microphone. ;)


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/browser/productseries.py
  lib/lp/translations/browser/tests/test_productserieslanguage_views.py
  lib/lp/translations/stories/productseries/xx-productseries-translations.txt
  lib/lp/translations/templates/productseries-translations.pt


-- 
https://code.launchpad.net/~henninge/launchpad/devel-bug-638920-private-branch/+merge/39540
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/devel-bug-638920-private-branch into lp:launchpad/devel.