← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~danilo/launchpad/bug-872715 into lp:launchpad

 

Данило Шеган has proposed merging lp:~danilo/launchpad/bug-872715 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #872715 in Launchpad itself: "+configure-translations page includes builders configuration"
  https://bugs.launchpad.net/launchpad/+bug/872715

For more details, see:
https://code.launchpad.net/~danilo/launchpad/bug-872715/+merge/79099

= Bug 872715 =

Some changes to DistributionEditView have caused a few builder-related settings to appear on translation settings page like https://translations.launchpad.net/ubuntu/+configure-translations

== Implementation details ==

I expand out IServiceUsage settings in configure.zcml to be able to split out translations_usage into launchpad.TranslationsAdmin field (compared to the rest which are launchpad.Edit-restricted).  I considered making separate IServiceUsageWithoutTranslations interface as well, but as soon as we'd want to assign special privileges for blueprints_usage or similar, we'd end up with each field extracted into a separate interface, thus making the approach more cumbersome instead of less.

Similar is done for product settings.

For actual distribution view, the problem was that DistributionEditView.setUpFields was adding two fields which are not relevant.  I first started by overloading setUpFields in the translations form view, but decided that it's going to be more stable if I just make the view not depend on the DistributionEditView at all.

== Tests ==

bin/test -cvvt TestDistributionSettingsView -t TestProductTranslations -t TestDistributionTranslations

== Demo and Q/A ==

Go to

  https://translations.qastaging.launchpad.net/ubuntu/+configure-translations

as rosetta-expert (i.e. any LP team member) and make sure you can modify translations usage and that no builder fields show up.

Similarly, do the same for any product eg. https://translations.qastaging.launchpad.net/intltool/+configure-translations

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/configure.zcml
  lib/lp/registry/tests/test_distribution.py
  lib/lp/registry/tests/test_product.py
  lib/lp/translations/browser/distribution.py
  lib/lp/translations/browser/tests/test_distribution_views.py
  lib/lp/translations/browser/tests/test_product_view.py
-- 
https://code.launchpad.net/~danilo/launchpad/bug-872715/+merge/79099
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-872715 into lp:launchpad.
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-10-11 09:06:29 +0000
+++ lib/lp/registry/configure.zcml	2011-10-12 10:32:26 +0000
@@ -214,9 +214,6 @@
             permission="launchpad.Edit"
             set_schema="lp.app.interfaces.launchpad.IServiceUsage"/>
         <require
-            permission="launchpad.Edit"
-            interface="lp.registry.interfaces.distroseries.IDistroSeriesEditRestricted"/>
-        <require
             permission="launchpad.Admin"
             set_attributes="previous_series"/>
         <require
@@ -1197,7 +1194,8 @@
             interface="lp.registry.interfaces.product.IProductEditRestricted"/>
         <require
             permission="launchpad.Edit"
-            set_schema="lp.app.interfaces.launchpad.IServiceUsage"/>
+            set_attributes="answers_usage blueprints_usage codehosting_usage
+                            bug_tracking_usage uses_launchpad"/>
         <require
             permission="launchpad.Edit"
             set_attributes="
@@ -1262,7 +1260,8 @@
             set_attributes="
                 translation_focus
                 translationgroup
-                translationpermission"/>
+                translationpermission
+                translations_usage"/>
         <require
             permission="zope.Public"
             attributes="
@@ -1514,7 +1513,8 @@
             interface="lp.registry.interfaces.distribution.IDistributionEditRestricted"/>
         <require
             permission="launchpad.Edit"
-            set_schema="lp.app.interfaces.launchpad.IServiceUsage"/>
+            set_attributes="answers_usage blueprints_usage codehosting_usage
+                            bug_tracking_usage uses_launchpad"/>
         <require
             permission="launchpad.Moderate"
             interface="lp.registry.interfaces.distribution.IDistributionDriverRestricted"/>
@@ -1551,7 +1551,9 @@
                 language_pack_admin
                 translationgroup
                 translationpermission
-                translation_focus"/>
+                translation_focus
+                translations_usage
+                "/>
 
         <!-- IHasAliases -->
 

=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py	2011-08-25 13:10:45 +0000
+++ lib/lp/registry/tests/test_distribution.py	2011-10-12 10:32:26 +0000
@@ -24,6 +24,7 @@
     LaunchpadFunctionalLayer,
     ZopelessDatabaseLayer,
     )
+from lp.app.enums import ServiceUsage
 from lp.app.errors import NotFoundError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.errors import NoSuchDistroSeries
@@ -41,12 +42,14 @@
     IDistributionSourcePackageRelease,
     )
 from lp.testing import (
+    celebrity_logged_in,
     login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.matchers import Provides
 from lp.testing.views import create_initialized_view
+from lp.translations.enums import TranslationPermission
 
 
 class TestDistribution(TestCaseWithFactory):
@@ -491,3 +494,33 @@
         self.factory.makeDistroSeriesParent(derived_series=other_series)
         distroset = getUtility(IDistributionSet)
         self.assertEqual(1, len(list(distroset.getDerivedDistributions())))
+
+
+class TestDistributionTranslations(TestCaseWithFactory):
+    """A TestCase for accessing distro translations-related attributes."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_rosetta_expert(self):
+        distro = self.factory.makeDistribution()
+        new_series = self.factory.makeDistroSeries(distribution=distro)
+        group = self.factory.makeTranslationGroup()
+        with celebrity_logged_in('rosetta_experts'):
+            distro.translations_usage = ServiceUsage.LAUNCHPAD
+            distro.translation_focus = new_series
+            distro.translationgroup = group
+            distro.translationpermission = TranslationPermission.CLOSED
+
+    def test_translation_group_owner(self):
+        distro = self.factory.makeDistribution()
+        new_series = self.factory.makeDistroSeries(distribution=distro)
+        group = self.factory.makeTranslationGroup()
+        with celebrity_logged_in('rosetta_experts'):
+            distro.translationgroup = group
+
+        new_group = self.factory.makeTranslationGroup()
+        with person_logged_in(group.owner):
+            distro.translations_usage = ServiceUsage.LAUNCHPAD
+            distro.translation_focus = new_series
+            distro.translationgroup = new_group
+            distro.translationpermission = TranslationPermission.CLOSED

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2011-08-23 00:29:42 +0000
+++ lib/lp/registry/tests/test_product.py	2011-10-12 10:32:26 +0000
@@ -32,6 +32,7 @@
     )
 from lp.registry.model.productlicense import ProductLicense
 from lp.testing import (
+    celebrity_logged_in,
     login,
     login_person,
     TestCase,
@@ -355,6 +356,22 @@
             self.product.bug_supervisor.name))
 
 
+class TestProductTranslations(TestCaseWithFactory):
+    """A TestCase for accessing product translations-related attributes."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_rosetta_expert(self):
+        product = self.factory.makeProduct()
+        new_series = self.factory.makeProductSeries(product=product)
+        group = self.factory.makeTranslationGroup()
+        with celebrity_logged_in('rosetta_experts'):
+            product.translations_usage = ServiceUsage.LAUNCHPAD
+            product.translation_focus = new_series
+            product.translationgroup = group
+            product.translationpermission = TranslationPermission.CLOSED
+
+
 class TestWebService(WebServiceTestCase):
 
     def test_translations_usage(self):

=== modified file 'lib/lp/translations/browser/distribution.py'
--- lib/lp/translations/browser/distribution.py	2011-10-07 16:08:17 +0000
+++ lib/lp/translations/browser/distribution.py	2011-10-12 10:32:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Translations browser views for distributions."""
@@ -24,7 +24,7 @@
 from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.menu import NavigationMenu
 from lp.app.enums import service_uses_launchpad
-from lp.registry.browser.distribution import DistributionEditView
+from lp.registry.browser import RegistryEditFormView
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.propertycache import cachedproperty
@@ -128,9 +128,11 @@
                       reverse=True)
 
 
-class DistributionSettingsView(TranslationsMixin, DistributionEditView):
+class DistributionSettingsView(TranslationsMixin, RegistryEditFormView):
     label = "Translations settings"
     page_title = "Settings"
+    schema = IDistribution
+
     field_names = [
         "translations_usage",
         "translation_focus",
@@ -146,5 +148,4 @@
 
     @action('Change', name='change')
     def edit(self, action, data):
-        self.change_archive_fields(data)
         self.updateContextFromData(data)

=== added file 'lib/lp/translations/browser/tests/test_distribution_views.py'
--- lib/lp/translations/browser/tests/test_distribution_views.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/browser/tests/test_distribution_views.py	2011-10-12 10:32:26 +0000
@@ -0,0 +1,63 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the translations views on a distroseries."""
+
+__metaclass__ = type
+
+from zope.security.interfaces import Unauthorized
+
+from canonical.launchpad.webapp import canonical_url
+from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+from lp.testing.views import create_initialized_view
+
+
+class TestDistributionSettingsView(TestCaseWithFactory):
+    """Test distribution settings (+configure-translations) view."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def test_only_translation_fields(self):
+        distribution = self.factory.makeDistribution()
+        view = create_initialized_view(
+            distribution, '+configure-translations', rootsite='translations')
+        self.assertContentEqual(
+            ["translations_usage",
+             "translation_focus",
+             "translationgroup",
+             "translationpermission",
+             ],
+            view.field_names)
+
+    def test_unprivileged_users(self):
+        unprivileged = self.factory.makePerson()
+        distribution = self.factory.makeDistribution()
+        browser = self.getUserBrowser(user=unprivileged)
+        url = canonical_url(distribution, view_name='+configure-translations',
+                            rootsite='translations')
+        self.assertRaises(Unauthorized, browser.open, url)
+
+    def test_translation_group_owner(self):
+        # Translation group owner for a particular distribution has
+        # launchpad.TranslationsAdmin privileges on it.
+        group = self.factory.makeTranslationGroup()
+        distribution = self.factory.makeDistribution()
+        with person_logged_in(distribution.owner):
+            distribution.translationgroup = group
+        browser = self.getUserBrowser(user=group.owner)
+        url = canonical_url(distribution, view_name='+configure-translations',
+                            rootsite='translations')
+        # No "Unauthorized" exception is thrown.
+        browser.open(url)
+
+    def test_distribution_owner(self):
+        distribution = self.factory.makeDistribution()
+        browser = self.getUserBrowser(user=distribution.owner)
+        url = canonical_url(distribution, view_name='+configure-translations',
+                            rootsite='translations')
+        # No "Unauthorized" exception is thrown.
+        browser.open(url)

=== modified file 'lib/lp/translations/browser/tests/test_product_view.py'
--- lib/lp/translations/browser/tests/test_product_view.py	2011-10-10 12:07:40 +0000
+++ lib/lp/translations/browser/tests/test_product_view.py	2011-10-12 10:32:26 +0000
@@ -12,6 +12,7 @@
 from lp.registry.interfaces.series import SeriesStatus
 from lp.testing import (
     login_person,
+    celebrity_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.views import create_view
@@ -106,3 +107,10 @@
         login_person(product.owner)
         view = create_view(product, '+translations', layer=TranslationsLayer)
         self.assertEqual(True, view.can_configure_translations())
+
+    def test_can_configure_translations_rosetta_expert(self):
+        product = self.factory.makeProduct()
+        with celebrity_logged_in('rosetta_experts'):
+            view = create_view(product, '+translations',
+                               layer=TranslationsLayer)
+            self.assertEqual(True, view.can_configure_translations())