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

I have renamed the method.

I have also added a test for pt_BR, since the use of „_” in language code and as a separator for form fields can generated errors.

I have removed „user.ensure_login(self.client)” and leave it only of the first page loaded in this test.
Do we really need to call it after each page load?

I have renamed the variables and remove the comment. Many thanks for this review!

=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
--- lib/canonical/launchpad/javascript/translations/pofile.js 2010-04-12 00:39:34 +0000
+++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-04-12 14:49:09 +0000
@@ -374,23 +374,20 @@
                     return;
                 }
                 if (this.get('checked') == true) {
- // When checking the "Need review" checkbox the new
- // translation should be automatically checked.
- var select = Y.one('#' + translation_id + '_select');
- if (select !== null) {
- select.set('checked', true);
+ var new_translation_select = Y.one(
+ '#' + translation_id + '_select');
+ if (new_translation_select !== null) {
+ new_translation_select.set('checked', true);
                     }
                 } else {
- // When unchecking the "Need review" checkbox and the new
- // translation is empty, the current translation should be
- // automatically selected.
- var inputfield = Y.one('#' + translation_id);
- if (inputfield !== null && inputfield.get('value') == '') {
- var current_radio_id = translation_id.replace(
+ var new_translation_field = Y.one('#' + translation_id);
+ if (new_translation_field !== null &&
+ new_translation_field.get('value') == '') {
+ var current_select_id = translation_id.replace(
                            /_new$/, '_radiobutton');
- var current_radio = Y.one('#' + current_radio_id);
- if (current_radio !== null) {
- current_radio.set('checked', true);
+ var current_select = Y.one('#' + current_select_id);
+ if (current_select !== null) {
+ current_select.set('checked', true);
                        }
                     }
                 }

=== modified file 'lib/lp/translations/windmill/tests/test_pofile_translate.py'
--- lib/lp/translations/windmill/tests/test_pofile_translate.py 2010-04-12 00:39:34 +0000
+++ lib/lp/translations/windmill/tests/test_pofile_translate.py 2010-04-12 14:37:32 +0000
@@ -51,7 +51,7 @@
         # Check that the associated radio button is selected.
         client.asserts.assertChecked(id=radiobutton_id)

- def _check_reset_translation_select(
+ def _checkResetTranslationSelect(
         self, client, checkbox, singular_new_select, singular_current_select,
         singular_new_field=None, plural_new_select=None):
         """Checks that the new translation select radio buttons are checked
@@ -137,7 +137,29 @@
         singular_new_field = u'msgset_144_es_translation_0_new'
         singular_current_select = u'msgset_144_es_translation_0_radiobutton'
         plural_new_select = u'msgset_144_es_translation_1_new_select'
- self._check_reset_translation_select(
+ self._checkResetTranslationSelect(
+ client,
+ checkbox=checkbox,
+ singular_new_select=singular_new_select,
+ singular_new_field=singular_new_field,
+ singular_current_select=singular_current_select,
+ plural_new_select=plural_new_select)
+
+ # Go to the zoom in page for a pt_BR translation with plural forms.
+ # pt_BR is a language code using the same delimiter as HTTP form
+ # fields and are prone to errors.
+ self.client.open(
+ url='http://translations.launchpad.dev:8085/'
+ 'ubuntu/hoary/+source/evolution/+pots/'
+ 'evolution-2.2/pt_BR/15/+translate')
+ self.client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+
+ checkbox = u'msgset_144_force_suggestion'
+ singular_new_select = u'msgset_144_pt_BR_translation_0_new_select'
+ singular_new_field = u'msgset_144_pt_BR_translation_0_new'
+ singular_current_select = u'msgset_144_pt_BR_translation_0_radiobutton'
+ plural_new_select = u'msgset_144_pt_BR_translation_1_new_select'
+ self._checkResetTranslationSelect(
             client,
             checkbox=checkbox,
             singular_new_select=singular_new_select,
@@ -151,12 +173,11 @@
                 'ubuntu/hoary/+source/evolution/+pots/'
                 'evolution-2.2/es/19/+translate')
         self.client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
- user.ensure_login(self.client)

         checkbox = u'msgset_148_force_suggestion'
         singular_new_select = u'msgset_148_es_translation_0_new_select'
         singular_current_select = u'msgset_148_es_translation_0_radiobutton'
- self._check_reset_translation_select(
+ self._checkResetTranslationSelect(
             client,
             checkbox=checkbox,
             singular_new_select=singular_new_select,
@@ -168,12 +189,11 @@
                 'ubuntu/hoary/+source/evolution/+pots/'
                 'evolution-2.2/es/+translate')
         self.client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
- user.ensure_login(self.client)

         checkbox = u'msgset_130_force_suggestion'
         singular_new_select = u'msgset_130_es_translation_0_new_select'
         singular_current_select = u'msgset_130_es_translation_0_radiobutton'
- self._check_reset_translation_select(
+ self._checkResetTranslationSelect(
             client,
             checkbox=checkbox,
             singular_new_select=singular_new_select,