Launchpad itself

Merge lp:~adiroiban/launchpad/bug-340662 into lp:launchpad

  1. bug-340662
  2. Merge into devel
Proposed by Adi Roiban
Status: Merged
Merged at revision: not available
Proposed branch: lp:~adiroiban/launchpad/bug-340662
Merge into: lp:launchpad
Diff against target: 222 lines (+63/-24)
6 files modified
lib/canonical/launchpad/security.py (+26/-1)
lib/lp/translations/browser/potemplate.py (+3/-2)
lib/lp/translations/configure.zcml (+8/-2)
lib/lp/translations/stories/productseries/xx-productseries-templates.txt (+21/-8)
lib/lp/translations/stories/standalone/xx-potemplate-edit.txt (+4/-10)
lib/lp/translations/templates/object-templates.pt (+1/-1)
To merge this branch: bzr merge lp:~adiroiban/launchpad/bug-340662
Reviewer Review Type Date Requested Status
Данило Шеган (community) Disapprove
Eleanor Berger (community) Approve
Review via email: mp+16629@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Adi Roiban (adiroiban) wrote :

= Bug 340662=
At the moment, "change details" link on POTemplates has very limited options.

We should move at least translation domain, path and "accept translations" from "Admin" to "Change details" form.

== Proposed fix ==

Add template name, translation domain, path and iscurrent (accept translations) to the +edit page for potemplate and potemplates subset.

== Pre-implementation notes ==
There are no pre-implementation notes.

== Implementation details ==

launchpad.Edit for POTemplate and POTemplateSubset is a subset of launchpad.TranslationsAdmin To allow access to disabled templates for product owner or translations admins the URL traversal permission check was changed to launchpad.Edit.
Other users have no access to disabled templates

== Tests ==
lp-test -t productseries-templates

== Demo and Q/A ==
As a product owner (ex. <email address hidden>) for a project (ex. evolution) go to a template page for that project
https://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/

You should see the „Change details” that links to the +edit page

From this page you should be able to change template name, domain name, path, details, priority, owner and „accept translation”.

Change some values and then save them.

From the series +template page you will see „Edit” links for each template, including disabled templates:
https://translations.launchpad.dev/evolution/trunk/+templates

Disabling a template it should still be in the list (with a red background) and you can still edit it.

= 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/configure.zcml
  lib/lp/translations/browser/potemplate.py
  lib/lp/translations/stories/productseries/xx-productseries-templates.txt
  lib/lp/translations/stories/standalone/xx-potemplate-edit.txt
  lib/lp/translations/templates/object-templates.pt

Revision history for this message
Eleanor Berger (intellectronica) :
review: Approve
Revision history for this message
Данило Шеган (danilo) wrote :

There are a bunch of problems with this branch. One is that there was no pre-implementation call/chat; eg. we should definitely not have allowed template name to be modified by everyone. That can cause serious system inconsistencies especially with message sharing.

Next, there are other serious problems with EditPOTemplateSubset permission. For instance, it calls EditByOwnersOrAdmins(IPOTemplateSubset).checkAuthenticated, where IPOTemplateSubset doesn't implement IHasOwner, as seen in bug #507498.

Adi, can I ask you please to prepare a branch to back out the template name from the list of launchpad.Edit editable attributes?

review: Disapprove

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 2009-12-24 13:33:41 +0000
3+++ lib/canonical/launchpad/security.py 2009-12-29 18:24:17 +0000
4@@ -1164,6 +1164,8 @@
5 self, user)
6
7
8+# Please keep EditPOTemplateSubset in sync with this, unless you
9+# know exactly what you are doing.
10 class EditPOTemplateDetails(AdminPOTemplateDetails, EditByOwnersOrAdmins):
11 permission = 'launchpad.Edit'
12 usedfor = IPOTemplate
13@@ -1645,7 +1647,7 @@
14 user.inTeam(celebs.bazaar_experts))
15
16
17-# Please keep this in sync with AdminPOTemplateDetails. Note that
18+# Please keep this in sync with AdminPOTemplateDetails. Note that
19 # this permission controls access to browsing into individual
20 # potemplates, but it's on a different object (POTemplateSubset)
21 # from AdminPOTemplateDetails, even though it looks almost identical
22@@ -1672,6 +1674,29 @@
23 return OnlyRosettaExpertsAndAdmins.checkAuthenticated(self, user)
24
25
26+# Please keep EditPOTemplate in sync with this, unless you
27+# know exactly what you are doing. Note that this permission controls
28+# access to browsing into individual potemplates, but it's on a different
29+# object (POTemplateSubset) from EditPOTemplateDetails,
30+# even though it looks almost identical
31+class EditPOTemplateSubset(AuthorizationBase):
32+ permission = 'launchpad.Edit'
33+ usedfor = IPOTemplateSubset
34+
35+ def checkAuthenticated(self, user):
36+ """Allow anyone with admin rights; owners, product owners and
37+ distribution owners; and for distros, translation group owners.
38+ """
39+ if (self.obj.productseries is not None and
40+ user.inTeam(self.obj.productseries.product.owner)):
41+ # The user is the owner of the product.
42+ return True
43+
44+ return (
45+ AdminPOTemplateSubset(self.obj).checkAuthenticated(user) or
46+ EditByOwnersOrAdmins(self.obj).checkAuthenticated(user))
47+
48+
49 class AdminDistroSeriesTranslations(AuthorizationBase):
50 permission = 'launchpad.TranslationsAdmin'
51 usedfor = IDistroSeries
52
53=== modified file 'lib/lp/translations/browser/potemplate.py'
54--- lib/lp/translations/browser/potemplate.py 2009-12-22 10:28:45 +0000
55+++ lib/lp/translations/browser/potemplate.py 2009-12-29 18:24:17 +0000
56@@ -512,7 +512,8 @@
57 """View class that lets you edit a POTemplate object."""
58
59 schema = IPOTemplate
60- field_names = ['description', 'priority', 'owner']
61+ field_names = ['name', 'translation_domain', 'description', 'priority',
62+ 'path', 'owner', 'iscurrent']
63 label = 'Edit translation template details'
64 page_title = 'Edit details'
65
66@@ -712,7 +713,7 @@
67 raise AssertionError('Unknown context for %s' % potemplate.title)
68
69 if ((official_rosetta and potemplate.iscurrent) or
70- check_permission('launchpad.TranslationsAdmin', self.context)):
71+ check_permission('launchpad.Edit', self.context)):
72 # The target is using officially Launchpad Translations and the
73 # template is available to be translated, or the user is a is a
74 # Launchpad administrator in which case we show everything.
75
76=== modified file 'lib/lp/translations/configure.zcml'
77--- lib/lp/translations/configure.zcml 2009-12-10 14:25:16 +0000
78+++ lib/lp/translations/configure.zcml 2009-12-29 18:24:17 +0000
79@@ -409,10 +409,16 @@
80 interface="lp.translations.interfaces.potemplate.IPOTemplate"/>
81 <require
82 permission="launchpad.Edit"
83- set_attributes="owner priority description"/>
84+ set_attributes="
85+ owner priority description translation_domain path
86+ iscurrent name"/>
87 <require
88 permission="launchpad.TranslationsAdmin"
89- set_attributes="name translation_domain productseries distroseries sourcepackagename sourcepackageversion binarypackagename languagepack path header iscurrent from_sourcepackagename date_last_updated source_file source_file_format"/>
90+ set_attributes="
91+ productseries distroseries sourcepackagename
92+ sourcepackageversion binarypackagename languagepack
93+ source_file_format source_file date_last_updated
94+ from_sourcepackagename header"/>
95 </class>
96 <adapter
97 provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
98
99=== modified file 'lib/lp/translations/stories/productseries/xx-productseries-templates.txt'
100--- lib/lp/translations/stories/productseries/xx-productseries-templates.txt 2009-11-30 18:18:08 +0000
101+++ lib/lp/translations/stories/productseries/xx-productseries-templates.txt 2009-12-29 18:24:17 +0000
102@@ -1,5 +1,3 @@
103-
104-
105 Templates view for ProductSeries
106 ================================
107
108@@ -21,8 +19,11 @@
109 >>> evolution_trunk = evolution.getSeries('trunk')
110 >>> template = factory.makePOTemplate(productseries=evolution_trunk,
111 ... name='at-the-top')
112+ >>> template = factory.makePOTemplate(productseries=evolution_trunk,
113+ ... name='disabled')
114+ >>> template.iscurrent = False
115 >>> logout()
116-
117+ >>> owner_browser = setupBrowser('Basic test@canonical.com:test')
118
119 Getting there
120 -------------
121@@ -41,6 +42,7 @@
122 -------------------
123
124 The page shows a table of all templates and links to their subpages.
125+Users will not see disabled templates.
126
127 >>> table = find_tag_by_id(user_browser.contents, 'templates_table')
128 >>> print extract_text(table)
129@@ -58,6 +60,7 @@
130 >>> print extract_text(table)
131 Template name Last update Actions
132 at-the-top ... Edit Upload Download Administer
133+ disabled (inactive) ... Edit Upload Download Administer
134 evolution-2.2 2005-08-25 Edit Upload Download Administer
135 evolution-2.2-test 2006-12-13 Edit Upload Download Administer
136
137@@ -72,11 +75,21 @@
138 >>> print admin_browser.url
139 http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2
140
141+Clicking on 'Admin' will take the user to the page to administer the template.
142+Likewise for the other links for each template.
143+
144+ >>> admin_browser.open(
145+ ... 'http://translations.launchpad.dev/evolution/trunk/+templates')
146+ >>> admin_browser.getLink('Admin', index=0).click()
147+ >>> print admin_browser.url
148+ http://translations.launchpad.dev/evolution/trunk/+pots/at-the-top/+admin
149+
150 Clicking on 'Edit' will take the user to the page to edit the template
151-details. Likewise for the other links for each template.
152+details, even if the translation of the templates are not active.
153
154- >>> admin_browser.open(
155+ >>> owner_browser.open(
156 ... 'http://translations.launchpad.dev/evolution/trunk/+templates')
157- >>> admin_browser.getLink('Edit').click()
158- >>> print admin_browser.url
159- http://translations.launchpad.dev/evolution/trunk/+pots/at-the-top/+edit
160+ >>> owner_browser.getLink('Edit', index=1).click()
161+ >>> print owner_browser.url
162+ http://translations.launchpad.dev/evolution/trunk/+pots/disabled/+edit
163+
164
165=== modified file 'lib/lp/translations/stories/standalone/xx-potemplate-edit.txt'
166--- lib/lp/translations/stories/standalone/xx-potemplate-edit.txt 2009-09-17 10:22:14 +0000
167+++ lib/lp/translations/stories/standalone/xx-potemplate-edit.txt 2009-12-29 18:24:17 +0000
168@@ -20,7 +20,7 @@
169 Traceback (most recent call last):
170 ...
171 LinkNotFoundError
172-
173+
174
175 On the other hand, Rosetta expert (Jordi) can reach the POTemplate edit
176 page.
177@@ -43,9 +43,7 @@
178 We should be sure that the edit form get only the non admin fields.
179
180 >>> browser.getControl(name='field.name').value
181- Traceback (most recent call last):
182- ...
183- LookupError:...
184+ 'evolution-2.2'
185 >>> browser.getControl(name='field.description').value
186 'Template for evolution in hoary'
187 >>> browser.getControl(name='field.header').value
188@@ -53,9 +51,7 @@
189 ...
190 LookupError:...
191 >>> browser.getControl(name='field.iscurrent').value
192- Traceback (most recent call last):
193- ...
194- LookupError:...
195+ True
196 >>> browser.getControl(name='field.owner').value
197 'rosetta-admins'
198 >>> browser.getControl(name='field.productseries').value
199@@ -83,9 +79,7 @@
200 ...
201 LookupError:...
202 >>> browser.getControl(name='field.path').value
203- Traceback (most recent call last):
204- ...
205- LookupError:...
206+ 'po/evolution-2.2.pot'
207
208 And that we are able to POST it.
209
210
211=== modified file 'lib/lp/translations/templates/object-templates.pt'
212--- lib/lp/translations/templates/object-templates.pt 2009-12-10 12:46:11 +0000
213+++ lib/lp/translations/templates/object-templates.pt 2009-12-29 18:24:17 +0000
214@@ -84,7 +84,7 @@
215 <tbody>
216 <tal:templates repeat="template view/iter_templates">
217 <tal:not-current condition="not: template/iscurrent">
218- <tal:admin condition="template/required:launchpad.TranslationsAdmin">
219+ <tal:admin condition="template/required:launchpad.Edit">
220 <tr class="template_row inactive-template">
221 <td tal:condition="view/is_distroseries"
222 tal:content="template/sourcepackagename/name"