Launchpad itself

Merge lp:~henninge/launchpad/bug-507498-oops-subset into lp:launchpad

  1. bug-507498-oops-subset
  2. Merge into devel
Proposed by Henning Eggers
Status: Rejected
Rejected by: Henning Eggers
Proposed branch: lp:~henninge/launchpad/bug-507498-oops-subset
Merge into: lp:launchpad
Diff against target: 40 lines (+18/-1)
2 files modified
lib/canonical/launchpad/security.py (+1/-1)
lib/lp/translations/doc/potemplate.txt (+17/-0)
To merge this branch: bzr merge lp:~henninge/launchpad/bug-507498-oops-subset
Reviewer Review Type Date Requested Status
Guilherme Salgado (community) code Approve
Review via email: mp+17459@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :

This is a simple fix for bug 507498 to stop the OOPS from happening. It fixes the persmission checker that tried to check the owner of a POTemplateSubset which it does not have. A somewhat articficial test was added to reproduce the error.

Revision history for this message
Guilherme Salgado (salgado) wrote :

Hi Henning,

Since your new test is a test for a corner case, I'd rather see it as a unit test (e.g. in lp/translations/tests/test_potemplate.py) than in a doctest.

review: Approve (code)
Revision history for this message
Adi Roiban (adiroiban) wrote :

Just a small comment:

Since AdminPOTemplateSubset(self.obj).checkAuthenticated(user) is checking for user.is_admin, it can be left out and have only:

return AdminPOTemplateSubset(self.obj).checkAuthenticated(user)

Revision history for this message
Henning Eggers (henninge) wrote :

This will not be merged as Adi is working on a branch that fixes this bug, too.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/security.py'
2--- lib/canonical/launchpad/security.py 2010-01-14 11:36:51 +0000
3+++ lib/canonical/launchpad/security.py 2010-01-15 14:04:15 +0000
4@@ -1668,7 +1668,7 @@
5
6 return (
7 AdminPOTemplateSubset(self.obj).checkAuthenticated(user) or
8- EditByOwnersOrAdmins(self.obj).checkAuthenticated(user))
9+ user.in_admin)
10
11
12 class AdminDistroSeriesTranslations(AuthorizationBase):
13
14=== modified file 'lib/lp/translations/doc/potemplate.txt'
15--- lib/lp/translations/doc/potemplate.txt 2009-11-01 22:50:17 +0000
16+++ lib/lp/translations/doc/potemplate.txt 2010-01-15 14:04:15 +0000
17@@ -210,6 +210,23 @@
18 True
19
20
21+== launchpd.Edit permissions on a POTemplateSubset ==
22+
23+A normal unprivileged user does not have edit rights for a subset. If this
24+test seems artificial, note that it is to verify the fix for bug 507498.
25+
26+ >>> unprivperson = factory.makePerson()
27+ >>> distroseries = factory.makeDistroSeries()
28+ >>> sourcepackage = factory.makeSourcePackage(distroseries=distroseries)
29+ >>> subset = potemplate_set.getSubset(
30+ ... distroseries=distroseries,
31+ ... sourcepackagename=sourcepackage.sourcepackagename)
32+ >>> from canonical.launchpad.webapp.authorization import check_permission
33+ >>> login_person(unprivperson)
34+ >>> print check_permission('launchpad.Edit', subset)
35+ False
36+
37+
38 = POTemplate =
39
40 POTemplate is an object with all strings that must be translated for a