Code review comment for lp:~adiroiban/launchpad/bug-487137

> Hi Adi!
>
> Thanks for yet another branch. The test is failing for me locally, and I've a
> few typo fixes below. Once they're fixed up, r=me.

Thanks for taking the time and review this branch!
Please see my inline comments.

[snip]

> > lp-test custom-language-codes
>
> Running the test results in a failure:
> http://pastebin.ubuntu.com/420372/

Test fixed.
It was my fault for changing the test and not running it again.

> > == Demo and Q/A ==
> >
> > Login as Rosetta Administrator.
> > Go to a product or sourcepackage page:
> >
> > https://translations.launchpad.dev/evolution
> > https://translations.launchpad.dev/ubuntu/+source/evolution
> >
> > You should see a link for defining custom language codes.
> > Click on the link.
> > Add a language code.
> > Click on the new language code.
> > Remove the language code.
>
> Great. Strangely, reverting your security.py change and logging in as
> <email address hidden> still allows me to create language codes, just not delete them.

Yes. It should work for IProduct since Rosetta Experts have already translations admin right on IProduct which implements IHasCustomLangaugeCode. They don't have this right on ISourcePackage which also implement IHasCustomLangaugeCode.

> I see it's not really part of this branch, but when removing a language there
> is no text (other than the main heading, breadcrumbs and the remove or cancel
> options). It looks a bit strange, and any text would just be repeating what
> the main heading already says, but adding the label to the form might look
> less strange... see what you think (as it's not part of this branch, I'll
> leave it up to you).

I have added the text:
You are going to remove the custom language code "LC".

> > === modified file 'lib/canonical/launchpad/security.py'
> > --- lib/canonical/launchpad/security.py 2010-04-18 22:31:40 +0000
> > +++ lib/canonical/launchpad/security.py 2010-04-22 03:19:15 +0000
> > @@ -112,6 +112,8 @@
> > from lp.blueprints.interfaces.sprintspecification import (
> > ISprintSpecification)
> > from lp.registry.interfaces.teammembership import ITeamMembership
> > +from lp.translations.interfaces.customlanguagecode import (
> > + ICustomLanguageCode, IHasCustomLanguageCodes)
> > from lp.translations.interfaces.translationgroup import (
> > ITranslationGroup, ITranslationGroupSet)
> > from lp.translations.interfaces.translationimportqueue import (
> > @@ -1652,6 +1654,28 @@
> > usedfor = ILanguage
> >
> >
> > +class AdminCustomLanguageCodes(OnlyRosettaExpertsAndAdmins):
> > + """Controls administration of custom language codes.
> > +
> > + Rosetta expters and Launchpad administrators can admister custom
> language
>
> s/expters/experts
> s/admister/administer

Fixed.

> > + codes.
> > + """
> > +
> > + permission = 'launchpad.TranslationsAdmin'
> > + usedfor = IHasCustomLanguageCodes
> > +
> > +
> > +class AdminCustomLanguageCode(OnlyRosettaExpertsAndAdmins):
> > + """Controls administration for a custom language code.
> > +
> > + Rosetta expters and Launchpad administrators can admister a custom
>
> as above.

Fixed.
[snip]
> > -An administrator sees the link to the custom language codes on a
> > -project's main translations page.
> > +An Launchpad administrator or Rosetta expert sees the link to the custom
>
> s/An/A

Fixed.

> > +language codes on a project's main translations page.
> >
> > >>> admin_browser.open(product_page)
> > - >>> tag = find_custom_language_codes_link(admin_browser)
> > + >>> tag = find_custom_language_codes_link(rosetta_admin_browser)
>
> I'm guessing this is the source of the test failure ^^^, I'm assuming you
> meant to find the custom language codes using the admin_browser here (which
> just opened the product page), as you test the rosetta admin browser below?

Yes. That was the problem.

[snip]

Here is the latest diff:

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py 2010-04-22 02:55:27 +0000
+++ lib/canonical/launchpad/security.py 2010-04-22 16:24:36 +0000
@@ -1657,8 +1657,8 @@
 class AdminCustomLanguageCodes(OnlyRosettaExpertsAndAdmins):
     """Controls administration of custom language codes.

- Rosetta expters and Launchpad administrators can admister custom language
- codes.
+ Rosetta experts and Launchpad administrators can administer custom
+ language codes.
     """

     permission = 'launchpad.TranslationsAdmin'
@@ -1668,7 +1668,7 @@
 class AdminCustomLanguageCode(OnlyRosettaExpertsAndAdmins):
     """Controls administration for a custom language code.

- Rosetta expters and Launchpad administrators can admister a custom
+ Rosetta experts and Launchpad administrators can administer a custom
     language code.
     """

=== modified file 'lib/lp/translations/browser/configure.zcml'
--- lib/lp/translations/browser/configure.zcml 2010-04-22 02:55:27 +0000
+++ lib/lp/translations/browser/configure.zcml 2010-04-22 16:21:23 +0000
@@ -1003,7 +1003,7 @@
         for="lp.translations.interfaces.customlanguagecode.ICustomLanguageCode"
         permission="launchpad.TranslationsAdmin"
         class="lp.translations.browser.customlanguagecode.CustomLanguageCodeRemoveView"
- template="../../app/templates/generic-edit.pt"
+ template="../templates/customlanguagecode-remove.pt"
         layer="canonical.launchpad.layers.TranslationsLayer"/>

 <!-- IHasCustomLanguageCodes -->

=== modified file 'lib/lp/translations/stories/standalone/custom-language-codes.txt'
--- lib/lp/translations/stories/standalone/custom-language-codes.txt 2010-04-22 03:16:53 +0000
+++ lib/lp/translations/stories/standalone/custom-language-codes.txt 2010-04-22 16:24:47 +0000
@@ -36,11 +36,11 @@
     >>> owner_browser = setupBrowser("Basic <email address hidden>:test")
     >>> rosetta_admin_browser = setupRosettaExpertBrowser()

-An Launchpad administrator or Rosetta expert sees the link to the custom
+A Launchpad administrator or Rosetta expert sees the link to the custom
 language codes on a project's main translations page.

     >>> admin_browser.open(product_page)
- >>> tag = find_custom_language_codes_link(rosetta_admin_browser)
+ >>> tag = find_custom_language_codes_link(admin_browser)
     >>> print extract_text(tag.renderContents())
     If necessary, you may
     define custom language codes

=== added file 'lib/lp/translations/templates/customlanguagecode-remove.pt'
--- lib/lp/translations/templates/customlanguagecode-remove.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/templates/customlanguagecode-remove.pt 2010-04-22 16:27:44 +0000
@@ -0,0 +1,18 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml"
+ xmlns:tal="http://xml.zope.org/namespaces/tal"
+ xmlns:metal="http://xml.zope.org/namespaces/metal"
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad">
+ <body>
+ <div metal:fill-slot="main">
+ <div metal:use-macro="context/@@launchpad_form/form">
+ <div metal:fill-slot="extra_info" class="documentDescription">
+ You are going to remove the custom language code
+ '<tal:language-code replace="view/code" />'.
+ </div>
+ </div>
+ </div>
+ </body>
+</html>