Adi Roiban wrote:
> Adi Roiban has proposed merging lp:~adiroiban/launchpad/bug-492375
> into lp:launchpad/devel.
> 
>     Requested reviews:
>     Canonical Launchpad Engineering (launchpad)
> Related bugs:
>   #492375 At first, display only translations statistics for preferred
>   languages
>   https://bugs.launchpad.net/bugs/492375
Hello Adi,

thank you very much for preparing this branch which generally looks
very good. Please see my comments below for more detail.

[..]

> == Tests ==
> 
> bin/test --layer=TranslationsWindmillLayer -D -t serieslanguages
All tests pass except for the one "that always fails" as discussed on IRC:
{{{
<adiroiban> al-maisan: ah... that is part of user.ensure_login(self.client)
<adiroiban> and it fails for all windmill tests
}}}

> == Demo and Q/A ==
> 
> Access one of the page presenting language series.
> For example http://translations.launchpad.dev/ubuntu
> 
> If you are not logged in, you will see a list of languages obtained
> using geoip.
> You can click the "View all languages" and the table should display
> all langauges for Ubuntu.
> 
> If you are logged in, it will show only your preferred languages.
This works as described.

> === modified file 'lib/canonical/launchpad/javascript/translations/translations.js'
> --- lib/canonical/launchpad/javascript/translations/translations.js	2009-11-24 09:30:01 +0000
> +++ lib/canonical/launchpad/javascript/translations/translations.js	2009-12-17 14:38:47 +0000
> @@ -116,7 +116,7 @@
>   * which is defined in a code fragment that is included in the page via TAL.
>   */
>  var init_status_choice = function(content_box, index, list) {
> -    content_box.setStyle('display', '')
> +    content_box.setStyle('display', '');
>      var conf = choice_confs[index];
>      conf.title = 'Change status to';
>      conf.contentBox = content_box;
> @@ -165,6 +165,11 @@
>      button_markers.on('click', show_output);
>  };
>  
> +var toggle_node_visibility = function(node, index, list) {
> +  node.toggleClass('unseen');
> +  node.toggleClass('seen');
> +};
> +
>  /**
>   * Set up the import queue page.
>   */
> @@ -183,6 +188,37 @@
>      spinner_loader.set('innerHTML', '');
>  };
>  
> +
> +/**
> + * Set up initial the visibility for languages in a serieslanguages table.
"Set up the initial visibility.." ?

> + */
> +translations.initialize_languages_table = function(Y) {
> +    Y.all('.not-preferred-language').each(function(node, index, list) {
> +        node.addClass('unseen');
> +    });
> +    Y.all('.preferred-language').each(function(node, index, list) {
> +        node.addClass('seen');
> +    });
> +};
> +
> +
> +/**
> + * Toggle visibility for languages in a serieslanguages table.
> + */
> +translations.toggle_languages_visibility = function(e) {
> +    e.preventDefault();
> +    Y.all('.not-preferred-language').each(toggle_node_visibility);
> +    var toggle_button = e.currentTarget;
> +    if (toggle_button.hasClass('all-languages-visible')) {
> +      toggle_button.setContent('View All Languages');
> +      toggle_button.removeClass('all-languages-visible');
> +    } else {
> +      toggle_button.setContent('View Only Preferred Languages');
> +      toggle_button.addClass('all-languages-visible');
> +    }
> +};
> +
> +
>  }, "0.1", {
>      // "oop" and "event" are required to fix known bugs in YUI, which
>      // are apparently fixed in a later version.

> === modified file 'lib/lp/translations/browser/distroseries.py'
> --- lib/lp/translations/browser/distroseries.py	2009-10-30 10:09:17 +0000
> +++ lib/lp/translations/browser/distroseries.py	2009-12-17 14:38:47 +0000
> @@ -107,7 +107,6 @@
>          else:
>              self.adminlabel = 'Settings for language packs'
>  
> -
>      @cachedproperty
>      def unused_language_packs(self):
>          unused_language_packs = helpers.shortlist(self.context.language_packs)
> @@ -236,6 +235,14 @@
>  
>          return sorted(distroserieslangs, key=lambda a: a.language.englishname)
>  
> +    def isPreferredLanguage(self, language):
> +        # if there are no preferred languages, mark all
> +        # languages as preferred
> +        if (len(self.translatable_languages) == 0):
> +            return True
> +        else:
> +            return language in self.translatable_languages
> +
>      @property
>      def potemplates(self):
>          return list(self.context.getCurrentTranslationTemplates())
> @@ -245,6 +252,7 @@
>          """Is this DistroSeries the translation focus."""
>          return self.context.distribution.translation_focus == self.context
>  
> +
>  class DistroSeriesTranslationsMenu(NavigationMenu):
>  
>      usedfor = IDistroSeries

> === modified file 'lib/lp/translations/browser/productseries.py'
> --- lib/lp/translations/browser/productseries.py	2009-12-10 10:46:53 +0000
> +++ lib/lp/translations/browser/productseries.py	2009-12-17 14:38:47 +0000
> @@ -57,6 +57,7 @@
>  
>  class ProductSeriesTranslationsMenuMixIn:
>      """Translation menu for `IProductSeries`."""
> +
>      def overview(self):
>          """Return a link to the overview page."""
>          return Link('', 'Overview')
> @@ -275,7 +276,7 @@
>                          warning = (
>                              "A file could not be uploaded because its "
>                              "name matched multiple existing uploads, for "
> -                            "different templates." )
> +                            "different templates.")
>                          ul_conflicts = (
>                              "The conflicting file name was:<br /> "
>                              "<ul><li>%s</li></ul>" % cgi.escape(conflicts[0]))
> @@ -378,6 +379,14 @@
>          return sorted(productserieslangs,
>                        key=lambda a: a.language.englishname)
>  
> +    def isPreferredLanguage(self, language):
> +        # if there are no preferred languages, mark all
> +        # languages as preferred
> +        if (len(self.translatable_languages) == 0):
> +            return True
> +        else:
> +            return language in self.translatable_languages
having isPreferredLanguage() in the code twice is less than ideal but
from the conversation on IRC I understood
  - there was no good place to put it just once and re-use it
  - this will be addressed when a single serieslanguage view for both
    product and distro series is introduced (bug #496361).

> +
>      @property
>      def has_translation_documentation(self):
>          """Are there translation instructions for this product."""
> @@ -421,8 +430,7 @@
>      def change_settings_action(self, action, data):
>          """Change the translation settings."""
>          if (self.context.translations_autoimport_mode !=
> -            data['translations_autoimport_mode']
> -            ):
> +            data['translations_autoimport_mode']):
>              self.updateContextFromData(data)
>              # Request an initial upload of translation files.
>              getUtility(IRosettaUploadJobSource).create(

> === modified file 'lib/lp/translations/templates/distribution-translations.pt'
> --- lib/lp/translations/templates/distribution-translations.pt	2009-11-07 16:14:22 +0000
> +++ lib/lp/translations/templates/distribution-translations.pt	2009-12-17 14:38:47 +0000
> @@ -1,13 +1,19 @@
> -
>  <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"
> ->
> +  metal:use-macro="view/macro:page/main_only">
>  
>    <body>
> +
> +  <div metal:fill-slot="head_epilogue">
> +    <metal:translations-js
> +      use-macro="context/@@+translations-macros/translations-js" />
> +    <metal:languages-table-js
> +      use-macro="context/@@+translations-macros/languages-table-js" />
> +  </div>
> +
>    <div metal:fill-slot="main">
>      <div class="translation-help-links">
>        <a href="https://help.launchpad.net/Translations"

> === modified file 'lib/lp/translations/templates/distroseries-langchart.pt'
> --- lib/lp/translations/templates/distroseries-langchart.pt	2009-07-17 17:59:07 +0000
> +++ lib/lp/translations/templates/distroseries-langchart.pt	2009-12-17 14:38:47 +0000
> @@ -25,7 +25,18 @@
>      </thead>
>  
>      <tbody>
> -      <tr tal:repeat="drlang view/distroserieslanguages">
> +      <tal:loop tal:repeat="drlang view/distroserieslanguages">
> +        <tal:preferred
> +            condition="python: view.isPreferredLanguage(drlang.language)">
> +            <tal:set-class
> +                define="global series_language_class string:preferred-language" />
> +        </tal:preferred>
> +        <tal:not-preferred
> +            condition="not:python: view.isPreferredLanguage(drlang.language)">
> +            <tal:set-class
> +                define="global series_language_class string:not-preferred-language" />
> +        </tal:not-preferred>
> +      <tr tal:attributes="class series_language_class">
>          <td>
>            <a tal:attributes="href string:${drlang/fmt:url}"
>               tal:content="drlang/language/englishname">
> @@ -72,6 +83,10 @@
>            </tal:count>
>          </td>
>        </tr>
> +      </tal:loop>
>      </tbody>
>    </table>
> +  <metal:languages-table-actions
> +    use-macro="context/@@+translations-macros/languages-table-actions" />
> +
>  </tal:root>

> === modified file 'lib/lp/translations/templates/distroseries-translations.pt'
> --- lib/lp/translations/templates/distroseries-translations.pt	2009-10-23 06:42:09 +0000
> +++ lib/lp/translations/templates/distroseries-translations.pt	2009-12-17 14:38:47 +0000
> @@ -5,6 +5,14 @@
>    metal:use-macro="view/macro:page/main_only">
>  
>    <body>
> +
> +    <div metal:fill-slot="head_epilogue">
> +      <metal:translations-js
> +        use-macro="context/@@+translations-macros/translations-js" />
> +      <metal:languages-table-js
> +        use-macro="context/@@+translations-macros/languages-table-js" />
> +    </div>
> +
>      <div metal:fill-slot="main">
>        <div class="translation-help-links">
>          <a href="https://help.launchpad.net/Translations"

> === modified file 'lib/lp/translations/templates/product-translations.pt'
> --- lib/lp/translations/templates/product-translations.pt	2009-11-10 02:02:34 +0000
> +++ lib/lp/translations/templates/product-translations.pt	2009-12-17 14:38:47 +0000
> @@ -2,10 +2,17 @@
>    xmlns="http://www.w3.org/1999/xhtml"
>    xmlns:tal="http://xml.zope.org/namespaces/tal"
>    xmlns:metal="http://xml.zope.org/namespaces/metal"
> -  metal:use-macro="view/macro:page/main_only"
> ->
> +  metal:use-macro="view/macro:page/main_only">
>  
>    <body>
> +
> +  <div metal:fill-slot="head_epilogue">
> +    <metal:translations-js
> +      use-macro="context/@@+translations-macros/translations-js" />
> +    <metal:languages-table-js
> +      use-macro="context/@@+translations-macros/languages-table-js" />
> +  </div>
> +
>      <div metal:fill-slot="main"
>           tal:define="uses_translations view/uses_translations;
>                       admin_user context/required:launchpad.TranslationsAdmin">

> === modified file 'lib/lp/translations/templates/productseries-translations-languages.pt'
> --- lib/lp/translations/templates/productseries-translations-languages.pt	2009-11-03 05:50:48 +0000
> +++ lib/lp/translations/templates/productseries-translations-languages.pt	2009-12-17 14:38:47 +0000
> @@ -15,9 +15,18 @@
>            </tr>
>          </thead>
>          <tbody>
> -          <tr tal:repeat="language_stats view/productserieslanguages"
> -             tal:attributes="class
> -                 string:stats language-${language_stats/language/code}">
> +          <tal:loop tal:repeat="language_stats view/productserieslanguages">
> +            <tal:preferred
> +                condition="python: view.isPreferredLanguage(language_stats.language)">
> +                <tal:set-class
> +                    define="global series_language_class string:stats language-${language_stats/language/code} preferred-language" />
> +            </tal:preferred>
> +            <tal:not-preferred
> +                condition="not:python: view.isPreferredLanguage(language_stats.language)">
> +                <tal:set-class
> +                    define="global series_language_class string:stats language-${language_stats/language/code} not-preferred-language" />
> +            </tal:not-preferred>
> +          <tr tal:attributes="class series_language_class">
>              <td>
>                <tal:multiple_pofile condition="not:view/single_potemplate">
>                <a tal:attributes="href language_stats/fmt:url"
> @@ -68,6 +77,9 @@
>                </tal:block>
>              </td>
>            </tr>
> +          </tal:loop>
>          </tbody>
>        </table>
> +      <metal:languages-table-actions
> +        use-macro="context/@@+translations-macros/languages-table-actions" />
>  </tal:root>

> === modified file 'lib/lp/translations/templates/productseries-translations.pt'
> --- lib/lp/translations/templates/productseries-translations.pt	2009-10-31 12:03:43 +0000
> +++ lib/lp/translations/templates/productseries-translations.pt	2009-12-17 14:38:47 +0000
> @@ -2,9 +2,17 @@
>    xmlns="http://www.w3.org/1999/xhtml"
>    xmlns:tal="http://xml.zope.org/namespaces/tal"
>    xmlns:metal="http://xml.zope.org/namespaces/metal"
> -  metal:use-macro="view/macro:page/main_only"
> ->
> +  metal:use-macro="view/macro:page/main_only">
> +
>    <body>
> +
> +    <div metal:fill-slot="head_epilogue">
> +      <metal:translations-js
> +        use-macro="context/@@+translations-macros/translations-js" />
> +      <metal:languages-table-js
> +        use-macro="context/@@+translations-macros/languages-table-js" />
> +    </div>
> +
>      <div metal:fill-slot="main">
>        <div class="translation-help-links">
>          <a href="https://help.launchpad.net/Translations"

> === modified file 'lib/lp/translations/templates/translations-macros.pt'
> --- lib/lp/translations/templates/translations-macros.pt	2009-12-07 18:42:21 +0000
> +++ lib/lp/translations/templates/translations-macros.pt	2009-12-17 14:38:47 +0000
> @@ -130,5 +130,40 @@
>  </metal:nav-pofile-subpages>
>  
>  
> +<metal:translations-js define-macro="translations-js">
> +      <script
> +        type="text/javascript"
> +        tal:condition="devmode"
> +        tal:attributes="src string:${icingroot}/build/translations/translations.js">
> +      </script>
> +</metal:translations-js>
> +
> +
> +<metal:languages-table-js define-macro="languages-table-js">
> +      <script type="text/javascript">
> +      LPS.use("node", "translations", function(Y) {
> +          Y.on("click", function(e) {
> +            Y.translations.toggle_languages_visibility(e);
> +          }, "#toggle-languages-visibility");
> +      });
> +      LPS.use( 'translations', 'event', function(Y) {
> +          Y.on('domready', function(e) {
> +              Y.translations.initialize_languages_table(Y);
> +          });
> +      });
> +      </script>
> +</metal:languages-table-js>
> +
> +
> +<metal:languages-table-actions define-macro="languages-table-actions">
> +  <div align="right">
> +    [ <a href="/+editmylanguages">Choose Preferred Languages...</a>
> +      &mdash;
> +      <a href="#" id="toggle-languages-visibility">
> +         View All Languages</a>
> +    ]
> +   </div>
> +</metal:languages-table-actions>
> +
>  </tal:root>
>  

> === modified file 'lib/lp/translations/windmill/tests/test_languages.py'
> --- lib/lp/translations/windmill/tests/test_languages.py	2009-12-08 11:19:21 +0000
> +++ lib/lp/translations/windmill/tests/test_languages.py	2009-12-17 14:38:47 +0000
> @@ -6,22 +6,19 @@
>  __metaclass__ = type
>  __all__ = []
>  
> -import transaction
> -
>  from windmill.authoring import WindmillTestClient
> -from zope.component import getUtility
>  
>  from canonical.launchpad.windmill.testing.constants import (
> -    FOR_ELEMENT, PAGE_LOAD, SLEEP)
> +    PAGE_LOAD, SLEEP)
>  from lp.translations.windmill.testing import TranslationsWindmillLayer
>  from lp.testing import TestCaseWithFactory
>  
> -INPUT_FIELD=(u"//div[contains(@class,'searchform')]"+
> +INPUT_FIELD = (u"//div[contains(@class,'searchform')]"+
>               u"//input[@id='field.search_lang']")
> -FILTER_BUTTON=(u"//div[contains(@class,'searchform')]"+
> +FILTER_BUTTON = (u"//div[contains(@class,'searchform')]"+
>                 u"//input[@value='Filter languages']")
> -LANGUAGE=u"//a[contains(@class, 'language') and text()='%s']/parent::li"
> -UNSEEN_VALIDATOR='className|unseen'
> +LANGUAGE = u"//a[contains(@class, 'language') and text()='%s']/parent::li"
> +UNSEEN_VALIDATOR = 'className|unseen'
>  
>  
>  class LanguagesFilterTest(TestCaseWithFactory):
> @@ -104,4 +101,3 @@
>              u'Mende': True,
>              u'French': True,
>              })
> -

> === added file 'lib/lp/translations/windmill/tests/test_serieslanguages.py'
> --- lib/lp/translations/windmill/tests/test_serieslanguages.py	1970-01-01 00:00:00 +0000
> +++ lib/lp/translations/windmill/tests/test_serieslanguages.py	2009-12-17 14:38:47 +0000
> @@ -0,0 +1,86 @@
> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Tests for series languages."""
> +
> +__metaclass__ = type
> +__all__ = []
> +
> +import transaction
> +
> +from windmill.authoring import WindmillTestClient
> +from zope.component import getUtility
> +
> +from canonical.launchpad.windmill.testing.constants import (
> +    FOR_ELEMENT, PAGE_LOAD, SLEEP)
> +from canonical.launchpad.windmill.testing import lpuser
> +from canonical.launchpad.windmill.testing.lpuser import login_person
> +from lp.translations.windmill.testing import TranslationsWindmillLayer
> +from lp.testing import TestCaseWithFactory
> +
> +LANGUAGE=(u"//table[@id='languagestats']/descendant::a[text()='%s']"
> +         u"/parent::td/parent::tr")
> +UNSEEN_VALIDATOR='className|unseen'
> +
> +
> +class LanguagesSeriesTest(TestCaseWithFactory):
> +    """Test for filtering preferred languages in serieslanguages table."""
> +
> +    layer = TranslationsWindmillLayer
> +
> +    def _toggle_languages_visiblity(self):
> +        self.client.click(id="toggle-languages-visibility")
> +        self.client.waits.sleep(milliseconds=SLEEP)
> +
> +    def _assert_languages_visible(self, languages):
> +        for language, visibility in languages.items():
> +            xpath = LANGUAGE % language
> +            if visibility:
> +                self.client.asserts.assertNotProperty(
> +                    xpath=xpath, validator=UNSEEN_VALIDATOR)
> +            else:
> +                self.client.asserts.assertProperty(
> +                    xpath=xpath, validator=UNSEEN_VALIDATOR)
> +
> +    def test_serieslanguages_table(self):
> +        """Test that filtering on the +languages page works.
> +
> +        The test cannot fully cover all languages on the page and so just
> +        tests three with a search string of 'de':
> +        German, because it's language code is 'de' but the names does not,
> +        Mende, because it contains a 'de' but the language code does not,
> +        French, because neither its name nor language code contain 'de'.
> +        """
> +        self.client = WindmillTestClient('SeriesLanguages Tables')
> +        start_url = 'http://translations.launchpad.dev:8085/ubuntu'
> +        user = lpuser.TRANSLATIONS_ADMIN
> +        # Go to the distribution languages page
> +        self.client.open(url=start_url)
> +        self.client.waits.forPageLoad(timeout=PAGE_LOAD)
> +        user.ensure_login(self.client)
> +
> +        # A link will be displayed for viewing all languages
> +        # and only user preferred langauges are displayed
> +        self.client.asserts.assertProperty(
> +            id=u'toggle-languages-visibility',
> +            validator='text|View All Languages')
> +        self._assert_languages_visible({
> +            u'Catalan': True,
> +            u'Spanish': True,
> +            u'French': False,
> +            u'Croatian': False,
> +            })
> +
> +        # Toggle language visibility by clicking the toggle link.
> +        self._toggle_languages_visiblity()
> +        self.client.asserts.assertProperty(
> +            id=u'toggle-languages-visibility',
> +            validator='text|View Only Preferred Languages')
> +        # All languages should be visible now
> +        self._assert_languages_visible({
> +            u'Catalan': True,
> +            u'Spanish': True,
> +            u'French': True,
> +            u'Croatian': True,
> +            })
> +

Best regards

-- 
Muharem Hrnjadovic <email address hidden>
Public key id   : B2BBFCFC
Key fingerprint : A5A3 CC67 2B87 D641 103F  5602 219F 6B60 B2BB FCFC