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

[Adi Roiban]

[Adi Roiban (adiroiban)]

= Bug 340662=
At the moment, "change details" link on POTemplates has very limited options.

We should move at least translation domain, path and "accept translations" from "Admin" to "Change details" form.

== Proposed fix ==

Add template name, translation domain, path and iscurrent (accept translations) to the +edit page for potemplate and potemplates subset.

== Pre-implementation notes ==
There are no pre-implementation notes.

== Implementation details ==

launchpad.Edit for POTemplate and POTemplateSubset is a subset of launchpad.TranslationsAdmin To allow access to disabled templates for product owner or translations admins the URL traversal permission check was changed to launchpad.Edit.
Other users have no access to disabled templates

== Tests ==
lp-test -t productseries-templates

== Demo and Q/A ==
As a product owner (ex. <email address hidden>) for a project (ex. evolution) go to a template page for that project
https://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/

You should see the „Change details” that links to the +edit page

From this page you should be able to change template name, domain name, path, details, priority, owner and „accept translation”.

Change some values and then save them.

From the series +template page you will see „Edit” links for each template, including disabled templates:
https://translations.launchpad.dev/evolution/trunk/+templates

Disabling a template it should still be in the list (with a red background) and you can still edit it.

= 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/security.py
  lib/lp/translations/configure.zcml
  lib/lp/translations/browser/potemplate.py
  lib/lp/translations/stories/productseries/xx-productseries-templates.txt
  lib/lp/translations/stories/standalone/xx-potemplate-edit.txt
  lib/lp/translations/templates/object-templates.pt

[Eleanor Berger (intellectronica)]

review: Approve

[Данило Шеган (danilo)]

There are a bunch of problems with this branch. One is that there was no pre-implementation call/chat; eg. we should definitely not have allowed template name to be modified by everyone. That can cause serious system inconsistencies especially with message sharing.

Next, there are other serious problems with EditPOTemplateSubset permission. For instance, it calls EditByOwnersOrAdmins(IPOTemplateSubset).checkAuthenticated, where IPOTemplateSubset doesn't implement IHasOwner, as seen in bug #507498.

Adi, can I ask you please to prepare a branch to back out the template name from the list of launchpad.Edit editable attributes?

review: Disapprove

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2009-12-24 13:33:41 +0000
+++ lib/canonical/launchpad/security.py	2009-12-29 18:24:17 +0000
@@ -1164,6 +1164,8 @@
                 self, user)
 
 
+# Please keep EditPOTemplateSubset in sync with this, unless you
+# know exactly what you are doing.
 class EditPOTemplateDetails(AdminPOTemplateDetails, EditByOwnersOrAdmins):
     permission = 'launchpad.Edit'
     usedfor = IPOTemplate
@@ -1645,7 +1647,7 @@
                 user.inTeam(celebs.bazaar_experts))
 
 
-# Please keep this in sync with AdminPOTemplateDetails.  Note that
+# Please keep this in sync with AdminPOTemplateDetails. Note that
 # this permission controls access to browsing into individual
 # potemplates, but it's on a different object (POTemplateSubset)
 # from AdminPOTemplateDetails, even though it looks almost identical
@@ -1672,6 +1674,29 @@
             return OnlyRosettaExpertsAndAdmins.checkAuthenticated(self, user)
 
 
+# Please keep EditPOTemplate in sync with this, unless you
+# know exactly what you are doing. Note that this permission controls
+# access to browsing into individual potemplates, but it's on a different
+# object (POTemplateSubset) from EditPOTemplateDetails,
+# even though it looks almost identical
+class EditPOTemplateSubset(AuthorizationBase):
+    permission = 'launchpad.Edit'
+    usedfor = IPOTemplateSubset
+
+    def checkAuthenticated(self, user):
+        """Allow anyone with admin rights; owners, product owners and
+        distribution owners; and for distros, translation group owners.
+        """
+        if (self.obj.productseries is not None and
+            user.inTeam(self.obj.productseries.product.owner)):
+            # The user is the owner of the product.
+            return True
+
+        return (
+            AdminPOTemplateSubset(self.obj).checkAuthenticated(user) or
+            EditByOwnersOrAdmins(self.obj).checkAuthenticated(user))
+
+
 class AdminDistroSeriesTranslations(AuthorizationBase):
     permission = 'launchpad.TranslationsAdmin'
     usedfor = IDistroSeries
 
=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py	2009-12-22 10:28:45 +0000
+++ lib/lp/translations/browser/potemplate.py	2009-12-29 18:24:17 +0000
@@ -512,7 +512,8 @@
     """View class that lets you edit a POTemplate object."""
 
     schema = IPOTemplate
-    field_names = ['description', 'priority', 'owner']
+    field_names = ['name', 'translation_domain', 'description', 'priority',
+        'path', 'owner', 'iscurrent']
     label = 'Edit translation template details'
     page_title = 'Edit details'
 
@@ -712,7 +713,7 @@
             raise AssertionError('Unknown context for %s' % potemplate.title)
 
         if ((official_rosetta and potemplate.iscurrent) or
-            check_permission('launchpad.TranslationsAdmin', self.context)):
+            check_permission('launchpad.Edit', self.context)):
             # The target is using officially Launchpad Translations and the
             # template is available to be translated, or the user is a is a
             # Launchpad administrator in which case we show everything.
 
=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml	2009-12-10 14:25:16 +0000
+++ lib/lp/translations/configure.zcml	2009-12-29 18:24:17 +0000
@@ -409,10 +409,16 @@
                 interface="lp.translations.interfaces.potemplate.IPOTemplate"/>
             <require
                 permission="launchpad.Edit"
-                set_attributes="owner priority description"/>
+                set_attributes="
+                    owner priority description translation_domain path
+                    iscurrent name"/>
             <require
                 permission="launchpad.TranslationsAdmin"
-                set_attributes="name translation_domain productseries distroseries                                                                                                 sourcepackagename sourcepackageversion                                                                                                 binarypackagename languagepack path header                                                                                                 iscurrent from_sourcepackagename date_last_updated                                                                                                 source_file source_file_format"/>
+                set_attributes="
+                    productseries distroseries sourcepackagename 
+                    sourcepackageversion binarypackagename languagepack
+                    source_file_format source_file date_last_updated
+                    from_sourcepackagename header"/>
         </class>
         <adapter
            provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
 
=== modified file 'lib/lp/translations/stories/productseries/xx-productseries-templates.txt'
--- lib/lp/translations/stories/productseries/xx-productseries-templates.txt	2009-11-30 18:18:08 +0000
+++ lib/lp/translations/stories/productseries/xx-productseries-templates.txt	2009-12-29 18:24:17 +0000
@@ -1,5 +1,3 @@
-
-
 Templates view for ProductSeries
 ================================
 
@@ -21,8 +19,11 @@
     >>> evolution_trunk = evolution.getSeries('trunk')
     >>> template = factory.makePOTemplate(productseries=evolution_trunk,
     ...                                   name='at-the-top')
+    >>> template = factory.makePOTemplate(productseries=evolution_trunk,
+    ...                                   name='disabled')
+    >>> template.iscurrent = False
     >>> logout()
-
+    >>> owner_browser = setupBrowser('Basic [e-mail]:test')
 
 Getting there
 -------------
@@ -41,6 +42,7 @@
 -------------------
 
 The page shows a table of all templates and links to their subpages.
+Users will not see disabled templates.
 
     >>> table = find_tag_by_id(user_browser.contents, 'templates_table')
     >>> print extract_text(table)
@@ -58,6 +60,7 @@
     >>> print extract_text(table)
     Template name       Last update  Actions
     at-the-top          ...          Edit Upload Download Administer
+    disabled (inactive) ...          Edit Upload Download Administer
     evolution-2.2       2005-08-25   Edit Upload Download Administer
     evolution-2.2-test  2006-12-13   Edit Upload Download Administer
 
@@ -72,11 +75,21 @@
     >>> print admin_browser.url
     http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2
 
+Clicking on 'Admin' will take the user to the page to administer the template.
+Likewise for the other links for each template.
+
+    >>> admin_browser.open(
+    ...     'http://translations.launchpad.dev/evolution/trunk/+templates')
+    >>> admin_browser.getLink('Admin', index=0).click()
+    >>> print admin_browser.url
+    http://translations.launchpad.dev/evolution/trunk/+pots/at-the-top/+admin
+
 Clicking on 'Edit' will take the user to the page to edit the template
-details. Likewise for the other links for each template.
+details, even if the translation of the templates are not active.
 
-    >>> admin_browser.open(
+    >>> owner_browser.open(
     ...     'http://translations.launchpad.dev/evolution/trunk/+templates')
-    >>> admin_browser.getLink('Edit').click()
-    >>> print admin_browser.url
-    http://translations.launchpad.dev/evolution/trunk/+pots/at-the-top/+edit
+    >>> owner_browser.getLink('Edit', index=1).click()
+    >>> print owner_browser.url
+    http://translations.launchpad.dev/evolution/trunk/+pots/disabled/+edit
+
 
=== modified file 'lib/lp/translations/stories/standalone/xx-potemplate-edit.txt'
--- lib/lp/translations/stories/standalone/xx-potemplate-edit.txt	2009-09-17 10:22:14 +0000
+++ lib/lp/translations/stories/standalone/xx-potemplate-edit.txt	2009-12-29 18:24:17 +0000
@@ -20,7 +20,7 @@
   Traceback (most recent call last):
   ...
   LinkNotFoundError
-  
+
 
 On the other hand, Rosetta expert (Jordi) can reach the POTemplate edit
 page.
@@ -43,9 +43,7 @@
 We should be sure that the edit form get only the non admin fields.
 
   >>> browser.getControl(name='field.name').value
-  Traceback (most recent call last):
-  ...
-  LookupError:...
+  'evolution-2.2'
   >>> browser.getControl(name='field.description').value
   'Template for evolution in hoary'
   >>> browser.getControl(name='field.header').value
@@ -53,9 +51,7 @@
   ...
   LookupError:...
   >>> browser.getControl(name='field.iscurrent').value
-  Traceback (most recent call last):
-  ...
-  LookupError:...
+  True
   >>> browser.getControl(name='field.owner').value
   'rosetta-admins'
   >>> browser.getControl(name='field.productseries').value
@@ -83,9 +79,7 @@
   ...
   LookupError:...
   >>> browser.getControl(name='field.path').value
-  Traceback (most recent call last):
-  ...
-  LookupError:...
+  'po/evolution-2.2.pot'
 
 And that we are able to POST it.
 
 
=== modified file 'lib/lp/translations/templates/object-templates.pt'
--- lib/lp/translations/templates/object-templates.pt	2009-12-10 12:46:11 +0000
+++ lib/lp/translations/templates/object-templates.pt	2009-12-29 18:24:17 +0000
@@ -84,7 +84,7 @@
         <tbody>
           <tal:templates repeat="template view/iter_templates">
             <tal:not-current condition="not: template/iscurrent">
-              <tal:admin condition="template/required:launchpad.TranslationsAdmin">
+              <tal:admin condition="template/required:launchpad.Edit">
                 <tr class="template_row inactive-template">
                   <td tal:condition="view/is_distroseries"
                       tal:content="template/sourcepackagename/name"