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
=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py 2010-01-14 11:36:51 +0000
+++ lib/canonical/launchpad/security.py 2010-01-15 14:04:15 +0000
@@ -1668,7 +1668,7 @@
16681668
1669 return (1669 return (
1670 AdminPOTemplateSubset(self.obj).checkAuthenticated(user) or1670 AdminPOTemplateSubset(self.obj).checkAuthenticated(user) or
1671 EditByOwnersOrAdmins(self.obj).checkAuthenticated(user))1671 user.in_admin)
16721672
16731673
1674class AdminDistroSeriesTranslations(AuthorizationBase):1674class AdminDistroSeriesTranslations(AuthorizationBase):
16751675
=== modified file 'lib/lp/translations/doc/potemplate.txt'
--- lib/lp/translations/doc/potemplate.txt 2009-11-01 22:50:17 +0000
+++ lib/lp/translations/doc/potemplate.txt 2010-01-15 14:04:15 +0000
@@ -210,6 +210,23 @@
210 True210 True
211211
212212
213== launchpd.Edit permissions on a POTemplateSubset ==
214
215A normal unprivileged user does not have edit rights for a subset. If this
216test seems artificial, note that it is to verify the fix for bug 507498.
217
218 >>> unprivperson = factory.makePerson()
219 >>> distroseries = factory.makeDistroSeries()
220 >>> sourcepackage = factory.makeSourcePackage(distroseries=distroseries)
221 >>> subset = potemplate_set.getSubset(
222 ... distroseries=distroseries,
223 ... sourcepackagename=sourcepackage.sourcepackagename)
224 >>> from canonical.launchpad.webapp.authorization import check_permission
225 >>> login_person(unprivperson)
226 >>> print check_permission('launchpad.Edit', subset)
227 False
228
229
213= POTemplate =230= POTemplate =
214231
215POTemplate is an object with all strings that must be translated for a232POTemplate is an object with all strings that must be translated for a