Launchpad itself

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

  1. bug-497438
  2. Merge into devel
Proposed by Adi Roiban
Status: Merged
Approved by: Aaron Bentley
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~adiroiban/launchpad/bug-497438
Merge into: lp:launchpad
Diff against target: 357 lines (+108/-81)
8 files modified
lib/canonical/launchpad/security.py (+21/-5)
lib/canonical/launchpad/testing/pages.py (+28/-4)
lib/lp/registry/browser/distroseries.py (+2/-1)
lib/lp/registry/browser/sourcepackage.py (+2/-1)
lib/lp/translations/browser/distroseries.py (+1/-1)
lib/lp/translations/stories/distroseries/xx-distroseries-translations.txt (+22/-68)
lib/lp/translations/stories/productseries/xx-productseries-translations.txt (+31/-1)
lib/lp/translations/stories/standalone/xx-language.txt (+1/-0)
To merge this branch: bzr merge lp:~adiroiban/launchpad/bug-497438
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+16335@code.launchpad.net

Commit message

Use launchpad.TranslationsAdmin for checking access to distribution series translations.

To post a comment you must log in.
Revision history for this message
Adi Roiban (adiroiban) wrote :

= Bug 497438 =

While reviewing the import queue for Lucid, and as a member of rosetta-admins and ubuntu-translations-coordinators, I've noticed the following:

If I go to https://translations.edge.launchpad.net/ubuntu/lucid/+source/gfxboot-theme-ubuntu, I can see the list of templates for that package in my preferred languages.

However, when I try to access the template page at https://translations.edge.launchpad.net/ubuntu/lucid/+source/gfxboot-theme-ubuntu/+pots/bootloader, I get a "Translation page is not available. Translations for this release series are not available yet." error message and cannot access the page. The same happens when I try to access the +admin page.

== Implementation details ==
Since we are migrating translations permission from launchpad.Admin to launchpad.TranslationsAdmin, just replace launchpad.Admin with launchpad.TranslationsAdmin in the appropriate places.

The tests from distroseries-translations were switched from admin_browser to utc_browser, a browser configured for launchpad.TranslationsAdmin

Also there were some productseries tests and they were moved. Same for language specific tests.

After talking with Danilo, we agreed of having an utility (helper) function for setting up a browser configured for launchpad.TranslationsAdmin

== Tests ==

bin/test -vvct distroseries-translations

== Demo and Q/A ==
Login in Launchpad as RosettaExpert (carlos) or a member of distribution translation group.

Go to distroseries translation setting page and hide the translations
https://translations.launchpad.dev/ubuntu/hoary/+admin

You should still be able to browse distroseries translations:
https://translations.launchpad.dev/ubuntu/hoary
https://translations.launchpad.dev/ubuntu/hoary/+lang/es

Login as a normal user or anonymous. Visit those pages again.
You should see a message telling you the translations are not available yet.

= 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/testing/pages.py
  lib/lp/registry/browser/distroseries.py
  lib/lp/registry/browser/sourcepackage.py
  lib/lp/translations/stories/distroseries/xx-distroseries-translations.txt

== Pylint notices ==

lib/canonical/launchpad/testing/pages.py
    39: [F0401] Unable to import 'lazr.restful.testing.webservice' (No module named restful)

lib/lp/registry/browser/sourcepackage.py
    23: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)

I have not touched the lazr.restful part. Those lint warning were already there.

Revision history for this message
Aaron Bentley (abentley) wrote :

This looks pretty good. I asked for a different name for utc_browser because people looking at the code will assume UTC stands for Universal Coordinated Time, and it was renamed "dtc_browser", which I think is fine.

It seems that we must do extra work to ensure that Persons used in doctests have pre-determined credentials so that we can hardcode them when calling setupBrowser. It would be nice if we had an alternative to setupBrowser that accepted arbitrary Persons, but it's not required for this patch.

By assigning to ubuntu.translationgroup, setupUTCBrowser has side effects that mean repeated calls will invalidate previous values. I was willing to accept this, since repeated calls should not be necessary, but Adi decided this should be fixed before landing.

review: Needs Fixing (code)
Revision history for this message
Adi Roiban (adiroiban) wrote :
Download full text (6.3 KiB)

Here is the latest diff.

Many thanks for the review!

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py 2009-12-18 13:25:19 +0000
+++ lib/canonical/launchpad/security.py 2009-12-21 15:17:16 +0000
@@ -1678,7 +1678,7 @@
     def checkAuthenticated(self, user):
         """Is the user able to manage `IDistroSeries` translations.

- Disribution managers can also manage IDistroSeries
+ Distribution managers can also manage IDistroSeries
         """
         return (AdminDistributionTranslations(
             self.obj.distribution).checkAuthenticated(user))
@@ -1691,7 +1691,7 @@
     def checkAuthenticated(self, user):
         """Is the user able to manage `IDistroSeriesLanguage` translations.

- Disribution managers can also manage IDistroSeriesLanguage
+ Distribution managers can also manage IDistroSeriesLanguage
         """
         return (AdminDistroSeriesTranslations(
             self.obj.distroseries).checkAuthenticated(user))

=== modified file 'lib/canonical/launchpad/testing/pages.py'
--- lib/canonical/launchpad/testing/pages.py 2009-12-18 13:25:19 +0000
+++ lib/canonical/launchpad/testing/pages.py 2009-12-21 17:17:46 +0000
@@ -39,6 +39,7 @@
 from lazr.restful.testing.webservice import WebServiceCaller
 from lp.testing import ANONYMOUS, login, login_person, logout
 from lp.testing.factory import LaunchpadObjectFactory
+from lp.registry.interfaces.person import NameAlreadyTaken

 class UnstickyCookieHTTPCaller(HTTPCaller):
@@ -636,17 +637,24 @@
     return LaunchpadWebServiceCaller(consumer_key, access_token.key)

-def setupUTCBrowser():
- """Testbrowser configured for Ubuntu Translations Coordinators."""
+def setupDTCBrowser():
+ """Testbrowser configured for Distribution Translations Coordinators.

+ Ubuntu is the configured distribution.
+ """
     login('<email address hidden>')
- utg_member = LaunchpadObjectFactory().makePerson(
- <email address hidden>", password="test")
- utg = LaunchpadObjectFactory().makeTranslationGroup(owner=utg_member)
- ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
- ubuntu.translationgroup = utg
+ try:
+ dtg_member = LaunchpadObjectFactory().makePerson(
+ <email address hidden>", password="test")
+ except NameAlreadyTaken:
+ # We have already created the translations coordinator
+ pass
+ else:
+ dtg = LaunchpadObjectFactory().makeTranslationGroup(owner=dtg_member)
+ ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+ ubuntu.translationgroup = dtg
     logout()
- return setupBrowser(auth='Basic <email address hidden>:test')
+ return setupBrowser(auth='Basic <email address hidden>:test')

 def stop():
@@ -669,7 +677,7 @@
     test.globs['user_webservice'] = LaunchpadWebServiceCaller(
         'launchpad-library', 'nopriv-read-nonprivate')
     test.globs['setupBrowser'] = setupBrowser
- test.globs['setupUTCBrowser'] = setupUTCBrowser
+ test.globs['setupDTCBrowser'] = setupDTCBrowser
     test.globs['browser'] = setupBrowser()
     test.globs['anon_browser'] = setupBrowser()
     test.globs['user_browser'] = setupBrowser(...

Read more...

Revision history for this message
Aaron Bentley (abentley) wrote :

Thanks, this looks fine now.

review: Approve

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-12 06:32:12 +0000
3+++ lib/canonical/launchpad/security.py 2009-12-21 17:23:18 +0000
4@@ -1671,14 +1671,30 @@
5 return OnlyRosettaExpertsAndAdmins.checkAuthenticated(self, user)
6
7
8-class AdminDistroSeriesLanguage(OnlyRosettaExpertsAndAdmins):
9- permission = 'launchpad.Admin'
10+class AdminDistroSeriesTranslations(AuthorizationBase):
11+ permission = 'launchpad.TranslationsAdmin'
12+ usedfor = IDistroSeries
13+
14+ def checkAuthenticated(self, user):
15+ """Is the user able to manage `IDistroSeries` translations.
16+
17+ Distribution managers can also manage IDistroSeries
18+ """
19+ return (AdminDistributionTranslations(
20+ self.obj.distribution).checkAuthenticated(user))
21+
22+
23+class AdminDistroSeriesLanguage(AuthorizationBase):
24+ permission = 'launchpad.TranslationsAdmin'
25 usedfor = IDistroSeriesLanguage
26
27+ def checkAuthenticated(self, user):
28+ """Is the user able to manage `IDistroSeriesLanguage` translations.
29
30-class AdminDistroSeriesTranslations(OnlyRosettaExpertsAndAdmins):
31- permission = 'launchpad.TranslationsAdmin'
32- usedfor = IDistroSeries
33+ Distribution managers can also manage IDistroSeriesLanguage
34+ """
35+ return (AdminDistroSeriesTranslations(
36+ self.obj.distroseries).checkAuthenticated(user))
37
38
39 class BranchSubscriptionEdit(AuthorizationBase):
40
41=== modified file 'lib/canonical/launchpad/testing/pages.py'
42--- lib/canonical/launchpad/testing/pages.py 2009-10-16 17:14:42 +0000
43+++ lib/canonical/launchpad/testing/pages.py 2009-12-21 17:23:18 +0000
44@@ -28,7 +28,8 @@
45 from zope.testbrowser.testing import Browser
46 from zope.testing import doctest
47
48-from canonical.launchpad.interfaces import IOAuthConsumerSet, OAUTH_REALM
49+from canonical.launchpad.interfaces import (
50+ IOAuthConsumerSet, OAUTH_REALM, ILaunchpadCelebrities)
51 from canonical.launchpad.testing.systemdocs import (
52 LayeredDocFileSuite, SpecialOutputChecker, strip_prefix)
53 from canonical.launchpad.webapp import canonical_url
54@@ -38,6 +39,7 @@
55 from lazr.restful.testing.webservice import WebServiceCaller
56 from lp.testing import ANONYMOUS, login, login_person, logout
57 from lp.testing.factory import LaunchpadObjectFactory
58+from lp.registry.interfaces.person import NameAlreadyTaken
59
60
61 class UnstickyCookieHTTPCaller(HTTPCaller):
62@@ -350,9 +352,10 @@
63 #
64 # The CData class does not override slicing though, so by slicing
65 # node first, we're effectively turning it into a concrete unicode
66- # instance, which does not wrap the contents when its __unicode__()
67- # is called of course. We could remove the unicode() call
68- # here, but we keep it for consistency and clarity purposes.
69+ # instance, which does not wrap the contents when its
70+ # __unicode__() is called of course. We could remove the
71+ # unicode() call here, but we keep it for consistency and clarity
72+ # purposes.
73 result.append(unicode(node[:]))
74 elif isinstance(node, NavigableString):
75 result.append(unicode(node))
76@@ -634,6 +637,26 @@
77 return LaunchpadWebServiceCaller(consumer_key, access_token.key)
78
79
80+def setupDTCBrowser():
81+ """Testbrowser configured for Distribution Translations Coordinators.
82+
83+ Ubuntu is the configured distribution.
84+ """
85+ login('foo.bar@canonical.com')
86+ try:
87+ dtg_member = LaunchpadObjectFactory().makePerson(
88+ email="dtg-member@ex.com", password="test")
89+ except NameAlreadyTaken:
90+ # We have already created the translations coordinator
91+ pass
92+ else:
93+ dtg = LaunchpadObjectFactory().makeTranslationGroup(owner=dtg_member)
94+ ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
95+ ubuntu.translationgroup = dtg
96+ logout()
97+ return setupBrowser(auth='Basic dtg-member@ex.com:test')
98+
99+
100 def stop():
101 # Temporarily restore the real stdout.
102 old_stdout = sys.stdout
103@@ -654,6 +677,7 @@
104 test.globs['user_webservice'] = LaunchpadWebServiceCaller(
105 'launchpad-library', 'nopriv-read-nonprivate')
106 test.globs['setupBrowser'] = setupBrowser
107+ test.globs['setupDTCBrowser'] = setupDTCBrowser
108 test.globs['browser'] = setupBrowser()
109 test.globs['anon_browser'] = setupBrowser()
110 test.globs['user_browser'] = setupBrowser(
111
112=== modified file 'lib/lp/registry/browser/distroseries.py'
113--- lib/lp/registry/browser/distroseries.py 2009-12-09 19:47:23 +0000
114+++ lib/lp/registry/browser/distroseries.py 2009-12-21 17:23:18 +0000
115@@ -91,7 +91,8 @@
116 distroserieslang = distroserieslangset.getDummy(
117 self.context, lang)
118
119- if not check_permission('launchpad.Admin', distroserieslang):
120+ if not check_permission(
121+ 'launchpad.TranslationsAdmin', distroserieslang):
122 self.context.checkTranslationsViewable()
123
124 return distroserieslang
125
126=== modified file 'lib/lp/registry/browser/sourcepackage.py'
127--- lib/lp/registry/browser/sourcepackage.py 2009-12-03 17:14:05 +0000
128+++ lib/lp/registry/browser/sourcepackage.py 2009-12-21 17:23:18 +0000
129@@ -56,7 +56,8 @@
130 distroseries=self.context.distroseries,
131 sourcepackagename=self.context.sourcepackagename)
132
133- if not check_permission('launchpad.Admin', sourcepackage_pots):
134+ if not check_permission(
135+ 'launchpad.TranslationsAdmin', sourcepackage_pots):
136 self.context.distroseries.checkTranslationsViewable()
137
138 return sourcepackage_pots
139
140=== modified file 'lib/lp/translations/browser/distroseries.py'
141--- lib/lp/translations/browser/distroseries.py 2009-10-30 10:09:17 +0000
142+++ lib/lp/translations/browser/distroseries.py 2009-12-21 17:23:18 +0000
143@@ -195,7 +195,7 @@
144 hidden and the user is not one of the limited caste that is
145 allowed to access them.
146 """
147- if check_permission('launchpad.Admin', self.context):
148+ if check_permission('launchpad.TranslationsAdmin', self.context):
149 # Anyone with admin rights on this series passes. This
150 # includes Launchpad admins.
151 return
152
153=== modified file 'lib/lp/translations/stories/distroseries/xx-distroseries-translations.txt'
154--- lib/lp/translations/stories/distroseries/xx-distroseries-translations.txt 2009-10-23 16:17:40 +0000
155+++ lib/lp/translations/stories/distroseries/xx-distroseries-translations.txt 2009-12-21 17:23:18 +0000
156@@ -1,29 +1,9 @@
157-= Distribution series translations =
158+Distribution series translations
159+================================
160
161 This page shows a list of PO templates contained within all source
162 packages in a particular distibution series.
163
164-We are going to work with visible (Spanish) and non visible
165-(Spanish (Spain)) languages.
166-
167-Here we can see that Spanish (Spain) is not visible.
168-
169- >>> admin_browser.open(
170- ... 'http://translations.launchpad.dev/+languages/es_ES/+admin')
171- >>> print admin_browser.getControl('The English name').value
172- Spanish (Spain)
173- >>> admin_browser.getControl('Visible').selected
174- False
175-
176-But Spanish is visible.
177-
178- >>> admin_browser.open(
179- ... 'http://translations.launchpad.dev/+languages/es/+admin')
180- >>> print admin_browser.getControl('The English name').value
181- Spanish
182- >>> admin_browser.getControl('Visible').selected
183- True
184-
185 In this case, we're asking for the translation overview for Hoary.
186
187 >>> anon_browser.open('http://translations.launchpad.dev/ubuntu/hoary')
188@@ -65,22 +45,24 @@
189
190 ... but the link is available to administrators:
191
192- >>> admin_browser.open('http://translations.launchpad.dev/ubuntu/hoary')
193-
194- >>> admin_browser.getLink('Change settings').click()
195+ >>> dtc_browser = setupDTCBrowser()
196+
197+ >>> dtc_browser.open('http://translations.launchpad.dev/ubuntu/hoary')
198+
199+ >>> dtc_browser.getLink('Change settings').click()
200
201 Once the administrator hides all translations...
202
203- >>> admin_browser.getControl(
204+ >>> dtc_browser.getControl(
205 ... 'Hide translations for this release').selected = True
206- >>> admin_browser.getControl('Change').click()
207- >>> print admin_browser.url
208+ >>> dtc_browser.getControl('Change').click()
209+ >>> print dtc_browser.url
210 http://translations.launchpad.dev/ubuntu/hoary
211
212 ...a notice about the fact shows up on the overview page.
213
214 >>> notices = find_tags_by_class(
215- ... admin_browser.contents, 'visibility-notice')
216+ ... dtc_browser.contents, 'visibility-notice')
217 >>> for notice in notices:
218 ... print extract_text(notice)
219 Translations for this series are currently hidden.
220@@ -108,9 +90,9 @@
221
222 >>> user_browser.handleErrors = False
223
224-Although, an administrator can see the same page.
225+Translations administrator have access series with hidden translations.
226
227- >>> admin_browser.open(
228+ >>> dtc_browser.open(
229 ... 'http://translations.launchpad.dev/ubuntu/hoary/+lang/es')
230
231 Non existing languages are not viewable. English is a special case
232@@ -142,7 +124,7 @@
233 However, source package translations are still available to the
234 administrators.
235
236- >>> admin_browser.open(
237+ >>> dtc_browser.open(
238 ... 'http://translations.launchpad.dev/ubuntu/hoary/'
239 ... '+sources/evolution')
240
241@@ -150,52 +132,24 @@
242 distribution should be deferred. That option is set also from the same
243 form where we hide all translations and an admin is able to change it:
244
245- >>> admin_browser.open('http://translations.launchpad.dev/ubuntu/hoary')
246- >>> admin_browser.getLink('Change settings').click()
247- >>> admin_browser.getControl(
248+ >>> dtc_browser.open('http://translations.launchpad.dev/ubuntu/hoary')
249+ >>> dtc_browser.getLink('Change settings').click()
250+ >>> dtc_browser.getControl(
251 ... 'Defer translation imports').selected
252 False
253- >>> admin_browser.getControl(
254+ >>> dtc_browser.getControl(
255 ... 'Defer translation imports').selected = True
256- >>> admin_browser.getControl('Change').click()
257- >>> print admin_browser.url
258+ >>> dtc_browser.getControl('Change').click()
259+ >>> print dtc_browser.url
260 http://translations.launchpad.dev/ubuntu/hoary
261
262 Once the system accepts the submission, we can see such change applied.
263
264- >>> admin_browser.getLink('Change settings').click()
265- >>> admin_browser.getControl(
266+ >>> dtc_browser.getLink('Change settings').click()
267+ >>> dtc_browser.getControl(
268 ... 'Defer translation imports').selected
269 True
270
271 There are no visible user interface changes once this flag is changed. It
272 just prevents that the translation import script, which is executed by cron,
273 handle translation imports for this distro series.
274-
275-== Translation focus ==
276-
277-If translation focus is not set, there is no recommendation of what
278-release series should be translated.
279-
280- >>> login('admin@canonical.com')
281- >>> distribution = factory.makeDistribution(name='earthian')
282- >>> distroseries = factory.makeDistroRelease(
283- ... name='1.4', distribution=distribution)
284- >>> logout()
285- >>> print distribution.translation_focus
286- None
287- >>> admin_browser.open('http://translations.launchpad.dev/earthian/1.4')
288- >>> print find_tag_by_id(admin_browser.contents, 'translation-focus')
289- None
290-
291-If focus is set, nice explanatory text is displayed.
292-
293- >>> login('admin@canonical.com')
294- >>> focus_series = factory.makeDistroRelease(
295- ... name='1.6', distribution=distribution)
296- >>> distribution.translation_focus = focus_series
297- >>> logout()
298- >>> admin_browser.open('http://translations.launchpad.dev/earthian/1.4')
299- >>> print extract_text(
300- ... find_tag_by_id(admin_browser.contents, 'translation-focus'))
301- Launchpad currently recommends translating 1.6.
302
303=== renamed file 'lib/lp/translations/stories/standalone/xx-productseries-translations.txt' => 'lib/lp/translations/stories/productseries/xx-productseries-translations.txt'
304--- lib/lp/translations/stories/standalone/xx-productseries-translations.txt 2009-12-10 10:46:05 +0000
305+++ lib/lp/translations/stories/productseries/xx-productseries-translations.txt 2009-12-21 17:23:18 +0000
306@@ -114,7 +114,7 @@
307 >>> serbian_link['href']
308 u'/frobnicator/trunk/+lang/sr'
309
310-Upload page and Translations use
311+Upload page and translations use
312 --------------------------------
313
314 If the product a series belongs to is not configured to use Launchpad
315@@ -238,3 +238,33 @@
316 Translations...
317 Translations are exported daily to branch
318 ...
319+
320+
321+Translation focus
322+-----------------
323+
324+If translation focus is not set, there is no recommendation of what
325+release series should be translated.
326+
327+ >>> login('admin@canonical.com')
328+ >>> distribution = factory.makeDistribution(name='earthian')
329+ >>> distroseries = factory.makeDistroRelease(
330+ ... name='1.4', distribution=distribution)
331+ >>> logout()
332+ >>> print distribution.translation_focus
333+ None
334+ >>> admin_browser.open('http://translations.launchpad.dev/earthian/1.4')
335+ >>> print find_tag_by_id(admin_browser.contents, 'translation-focus')
336+ None
337+
338+If focus is set, nice explanatory text is displayed.
339+
340+ >>> login('admin@canonical.com')
341+ >>> focus_series = factory.makeDistroRelease(
342+ ... name='1.6', distribution=distribution)
343+ >>> distribution.translation_focus = focus_series
344+ >>> logout()
345+ >>> admin_browser.open('http://translations.launchpad.dev/earthian/1.4')
346+ >>> print extract_text(
347+ ... find_tag_by_id(admin_browser.contents, 'translation-focus'))
348+ Launchpad currently recommends translating 1.6.
349
350=== modified file 'lib/lp/translations/stories/standalone/xx-language.txt'
351--- lib/lp/translations/stories/standalone/xx-language.txt 2009-12-08 08:16:35 +0000
352+++ lib/lp/translations/stories/standalone/xx-language.txt 2009-12-21 17:23:18 +0000
353@@ -308,3 +308,4 @@
354 ...
355 NotFound:...
356
357+