Launchpad itself

Merge lp:~henninge/launchpad/bug-515680-status into lp:launchpad

  1. bug-515680-status
  2. Merge into devel
Proposed by Henning Eggers
Status: Merged
Approved by: Henning Eggers
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~henninge/launchpad/bug-515680-status
Merge into: lp:launchpad
Diff against target: 217 lines (+80/-48)
4 files modified
lib/canonical/launchpad/security.py (+6/-20)
lib/lp/translations/interfaces/translationimportqueue.py (+5/-9)
lib/lp/translations/model/translationimportqueue.py (+26/-14)
lib/lp/translations/tests/test_translationimportqueue.py (+43/-5)
To merge this branch: bzr merge lp:~henninge/launchpad/bug-515680-status
Reviewer Review Type Date Requested Status
Eleanor Berger (community) code Approve
Review via email: mp+18517@code.launchpad.net

Commit message

Translation importers cannot set their queue entry to Blocked now.

To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :

= Bug 515680 =

The goal of this branch is to restrict access to the BLOCKED status to project owners and drivers (and admins, of course). The uploader should not be able to set their entry to BLOCKED.

== Proposed fix ==

Update TranslationImportQueueEntry.canSetStatus to disallow importers to set BLOCKED which it currently allows.

== Implementation details ==

I used this oportunity to update some very specific "is*" methods to be called "canEdit" and "canAdmin" and behave accordingly. This also changed their use in security.py.

== Test ==

./bin/test -vvct TestCanSetStatus

== Demo/QA ==

As the member of a translation team but not the owner or driver of the project, upload a file to a project. Go to them import queue and check that "Blocked" is not available to you. Go there as the maintainer or driver of the project and you can set it.

As a corner case, upload a file as the maintainer, Blocked should still be available.

= Launchpad lint =

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

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/lp/translations/interfaces/translationimportqueue.py
  lib/lp/translations/model/translationimportqueue.py
  lib/lp/translations/tests/test_translationimportqueue.py

== Pylint notices ==

lib/lp/translations/interfaces/translationimportqueue.py
    10: [F0401] Unable to import 'lazr.enum' (No module named enum)
    20: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)
    21: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
    22: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)

Revision history for this message
Eleanor Berger (intellectronica) :
review: Approve (code)

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-20 20:36:13 +0000
3+++ lib/canonical/launchpad/security.py 2010-02-03 16:23:12 +0000
4@@ -1283,23 +1283,15 @@
5 self, user)
6
7
8-class AdminTranslationImportQueueEntry(OnlyRosettaExpertsAndAdmins):
9+class AdminTranslationImportQueueEntry(AuthorizationBase):
10 permission = 'launchpad.Admin'
11 usedfor = ITranslationImportQueueEntry
12
13 def checkAuthenticated(self, user):
14- if OnlyRosettaExpertsAndAdmins.checkAuthenticated(self, user):
15- return True
16-
17- # As a special case, the Ubuntu translation group owners can
18- # manage Ubuntu uploads.
19- if self.obj.isUbuntuAndIsUserTranslationGroupOwner(user.person):
20- return True
21-
22- return False
23-
24-
25-class EditTranslationImportQueueEntry(AdminTranslationImportQueueEntry):
26+ return self.obj.canAdmin(user)
27+
28+
29+class EditTranslationImportQueueEntry(AuthorizationBase):
30 permission = 'launchpad.Edit'
31 usedfor = ITranslationImportQueueEntry
32
33@@ -1307,13 +1299,7 @@
34 """Anyone who can admin an entry, plus its owner or the owner of the
35 product or distribution, can edit it.
36 """
37- if AdminTranslationImportQueueEntry.checkAuthenticated(self, user):
38- return True
39- if self.obj.isUserUploaderOrOwner(user.person):
40- return True
41-
42- return False
43-
44+ return self.obj.canEdit(user)
45
46 class AdminTranslationImportQueue(OnlyRosettaExpertsAndAdmins):
47 permission = 'launchpad.Admin'
48
49=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
50--- lib/lp/translations/interfaces/translationimportqueue.py 2009-11-17 09:50:33 +0000
51+++ lib/lp/translations/interfaces/translationimportqueue.py 2010-02-03 16:23:13 +0000
52@@ -266,15 +266,11 @@
53 required=False,
54 readonly=True))
55
56- def isUbuntuAndIsUserTranslationGroupOwner(self, user):
57- """Check for special Ubuntu Translation Group.
58-
59- Return true if the entry is targeted to Ubuntu and the user is in
60- the team owning the Ubuntu translation group.
61- """
62-
63- def isUserUploaderOrOwner(user):
64- """Check for entry uploader or series owner."""
65+ def canAdmin(roles):
66+ """Check if the user can administer this entry."""
67+
68+ def canEdit(roles):
69+ """Check if the user can edit this entry."""
70
71 def canSetStatus(new_status, user):
72 """Check if the user can set this new status."""
73
74=== modified file 'lib/lp/translations/model/translationimportqueue.py'
75--- lib/lp/translations/model/translationimportqueue.py 2010-01-26 15:25:53 +0000
76+++ lib/lp/translations/model/translationimportqueue.py 2010-02-03 16:23:13 +0000
77@@ -277,26 +277,37 @@
78 distroseries=self.distroseries,
79 sourcepackagename=self.sourcepackagename).one()
80
81- def isUbuntuAndIsUserTranslationGroupOwner(self, user):
82+ def canAdmin(self, roles):
83 """See `ITranslationImportQueueEntry`."""
84 # As a special case, the Ubuntu translation group owners can
85 # manage Ubuntu uploads.
86 if self.is_targeted_to_ubuntu:
87 group = self.distroseries.distribution.translationgroup
88- if group is not None and user.inTeam(group.owner):
89+ if group is not None and roles.inTeam(group.owner):
90 return True
91+ # Rosetta experts and admins can administer the entry.
92+ return roles.in_rosetta_experts or roles.in_admin
93+
94+ def _canEditExcludeImporter(self, roles):
95+ """All people that can edit the entry except the importer."""
96+ # Admin rights include edit rights.
97+ if self.canAdmin(roles):
98+ return True
99+ # The maintainer and the drivers can edit the entry.
100+ if self.productseries is not None:
101+ return (roles.isOwner(self.productseries.product) or
102+ roles.isOneOfDrivers(self.productseries))
103+ if self.distroseries is not None:
104+ return (roles.isOwner(self.distroseries.distribution) or
105+ roles.isOneOfDrivers(self.distroseries))
106 return False
107
108- def isUserUploaderOrOwner(self, user):
109+ def canEdit(self, roles):
110 """See `ITranslationImportQueueEntry`."""
111- roles = IPersonRoles(user)
112+ # The importer can edit the entry.
113 if roles.inTeam(self.importer):
114 return True
115- if self.productseries is not None:
116- return roles.isOwner(self.productseries.product)
117- if self.distroseries is not None:
118- return roles.isOwner(self.distroseries.distribution)
119- return False
120+ return self._canEditExcludeImporter(roles)
121
122 def canSetStatus(self, new_status, user):
123 """See `ITranslationImportQueueEntry`."""
124@@ -304,13 +315,11 @@
125 # Anonymous user cannot do anything.
126 return False
127 roles = IPersonRoles(user)
128- can_admin = (roles.in_admin or roles.in_rosetta_experts or
129- self.isUbuntuAndIsUserTranslationGroupOwner(user))
130 if new_status == RosettaImportStatus.APPROVED:
131 # Only administrators are able to set the APPROVED status, and
132 # that's only possible if we know where to import it
133 # (import_into not None).
134- return can_admin and self.import_into is not None
135+ return self.canAdmin(roles) and self.import_into is not None
136 if new_status == RosettaImportStatus.IMPORTED:
137 # Only rosetta experts are able to set the IMPORTED status, and
138 # that's only possible if we know where to import it
139@@ -320,8 +329,11 @@
140 if new_status == RosettaImportStatus.FAILED:
141 # Only rosetta experts are able to set the FAILED status.
142 return roles.in_admin or roles.in_rosetta_experts
143- # All other statuses can bset set by all authorized persons.
144- return self.isUserUploaderOrOwner(user) or can_admin
145+ if new_status == RosettaImportStatus.BLOCKED:
146+ # Importers are not allowed to set BLOCKED
147+ return self._canEditExcludeImporter(roles)
148+ # All other statuses can be set set by all authorized persons.
149+ return self.canEdit(roles)
150
151 def setStatus(self, new_status, user):
152 """See `ITranslationImportQueueEntry`."""
153
154=== modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
155--- lib/lp/translations/tests/test_translationimportqueue.py 2009-12-18 10:15:14 +0000
156+++ lib/lp/translations/tests/test_translationimportqueue.py 2010-02-03 16:23:13 +0000
157@@ -33,6 +33,8 @@
158 self.rosetta_experts = (
159 getUtility(ILaunchpadCelebrities).rosetta_experts)
160 self.productseries = self.factory.makeProductSeries()
161+ self.productseries.driver = self.factory.makePerson()
162+ self.productseries.product.driver = self.factory.makePerson()
163 self.uploaderperson = self.factory.makePerson()
164
165 def _switch_dbuser(self):
166@@ -84,11 +86,47 @@
167 # The uploader can set some statuses.
168 self._assertCanSetStatus(self.uploaderperson, self.entry,
169 # A B D F I NR
170- [False, True, True, False, False, True])
171-
172- def test_canSetStatus_owner(self):
173- # The owner gets the same permissions.
174- self._assertCanSetStatus(self.productseries.product.owner, self.entry,
175+ [False, False, True, False, False, True])
176+
177+ def test_canSetStatus_product_owner(self):
178+ # The owner (maintainer) of the product gets to set Blocked as well.
179+ owner = self.productseries.product.owner
180+ self._assertCanSetStatus(owner, self.entry,
181+ # A B D F I NR
182+ [False, True, True, False, False, True])
183+
184+ def test_canSetStatus_owner_and_uploader(self):
185+ # Corner case: Nothing changes if the maintainer is also the uploader.
186+ self.productseries.product.owner = self.uploaderperson
187+ self._assertCanSetStatus(self.uploaderperson, self.entry,
188+ # A B D F I NR
189+ [False, True, True, False, False, True])
190+
191+ def test_canSetStatus_driver(self):
192+ # The driver gets the same permissions as the maintainer.
193+ driver = self.productseries.driver
194+ self._assertCanSetStatus(driver, self.entry,
195+ # A B D F I NR
196+ [False, True, True, False, False, True])
197+
198+ def test_canSetStatus_driver_and_uploader(self):
199+ # Corner case: Nothing changes if the driver is also the uploader.
200+ self.productseries.driver = self.uploaderperson
201+ self._assertCanSetStatus(self.uploaderperson, self.entry,
202+ # A B D F I NR
203+ [False, True, True, False, False, True])
204+
205+ def test_canSetStatus_product_driver(self):
206+ # The driver of the product, too.
207+ driver = self.productseries.product.driver
208+ self._assertCanSetStatus(driver, self.entry,
209+ # A B D F I NR
210+ [False, True, True, False, False, True])
211+
212+ def test_canSetStatus_product_driver_and_uploader(self):
213+ # Corner case: Nothing changes if the driver is also the uploader.
214+ self.productseries.product.driver = self.uploaderperson
215+ self._assertCanSetStatus(self.uploaderperson, self.entry,
216 # A B D F I NR
217 [False, True, True, False, False, True])
218