launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02895
[Merge] lp:~henninge/launchpad/devel-705176-sharing-info-link into lp:launchpad
You have been requested to review the proposed merge of lp:~henninge/launchpad/devel-705176-sharing-info-link into lp:launchpad.
For more details, see:
https://code.launchpad.net/~henninge/launchpad/devel-705176-sharing-info-link/+merge/51891
= Summary =
This is the final step in adding sharing information to translation pages. It adds a link to each "Sharing Information" portlet which were introduced in the last branch. The link points to a yet-to-be-created page that will provide detailed information about translation sharing and a place to take the necessary configuration step. Thus the link is named differently depending on the user's permissions to configure these things.
Here are the screenshots used in the UI review.
http://people.canonical.com/~henninge/screenshots/sharing-information/sharing-details-info.png
http://people.canonical.com/~henninge/screenshots/sharing-information/sharing-details-edit.png
http://people.canonical.com/~henninge/screenshots/sharing-information/sharing-details-setup.png
== Proposed fix ==
I added a Mixin to be used in all the view classes that creates an HTML snippet for the link. This was far easier than putting it together in TAL. The views that use this mixing implement some methods to provide the specific objects.
== Pre-implementation notes ==
Same as before.
== Implementation details ==
A lot of the implementation is extending the test to work with different users to verify that the correct link text is displayed. One trick here was to make sure that the user's password is 'test' because that is what getViewBrowser expects.
== Tests ==
bin/test -vvcm lp.translations.browser.test.test_sharing_information
bin/test -vvct xx-potemplate-index.txt
== Demo and Q/A ==
Try these as a privileged and an unprivileged user. The link should be "Edit sharing details" and "View sharing details" respectively.
You will have to enable the feature:
translations.sharing_information.enabled
View sharing information for a product series here:
http://translations.launchpad.dev/evolution/trunk
View sharing information for a template here:
http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2
View sharing information for a pofile here:
http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es
This will not show the link!
Once on the other side, you'll see the equivalent links there.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/browser/potemplate.py
lib/lp/translations/browser/productseries.py
lib/lp/translations/browser/sourcepackage.py
lib/lp/translations/browser/translationsharing.py
lib/lp/translations/browser/tests/test_sharing_information.py
lib/lp/translations/stories/standalone/xx-potemplate-index.txt
lib/lp/translations/templates/potemplate-index.pt
lib/lp/translations/templates/productseries-translations.pt
lib/lp/translations/templates/sourcepackage-translations.pt
--
https://code.launchpad.net/~henninge/launchpad/devel-705176-sharing-info-link/+merge/51891
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/devel-705176-sharing-info-link into lp:launchpad.
=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py 2011-03-01 17:40:06 +0000
+++ lib/lp/translations/browser/potemplate.py 2011-03-08 11:06:01 +0000
@@ -71,6 +71,9 @@
from lp.services.worlddata.interfaces.language import ILanguageSet
from lp.translations.browser.poexportrequest import BaseExportView
from lp.translations.browser.translations import TranslationsMixin
+from lp.translations.browser.translationsharing import (
+ TranslationSharingDetailsMixin,
+ )
from lp.translations.interfaces.pofile import IPOFileSet
from lp.translations.interfaces.potemplate import (
IPOTemplate,
@@ -229,7 +232,8 @@
self.request.response.redirect('../+translations')
-class POTemplateView(LaunchpadView, TranslationsMixin):
+class POTemplateView(LaunchpadView,
+ TranslationsMixin, TranslationSharingDetailsMixin):
SHOW_RELATED_TEMPLATES = 4
@@ -373,6 +377,20 @@
obj, template = infos[0]
return template
+ def getTranslationTarget(self):
+ """See `TranslationSharingDetailsMixin`."""
+ if self.is_upstream_template:
+ return self.context.productseries
+ else:
+ return self.context.sourcepackage
+
+ def can_edit_sharing_details(self):
+ if self.is_upstream_template:
+ obj = self.context.productseries
+ else:
+ obj = self.context.distroseries
+ return check_permission('launchpad.Edit', obj)
+
class POTemplateUploadView(LaunchpadView, TranslationsMixin):
"""Upload translations and updated template."""
=== modified file 'lib/lp/translations/browser/productseries.py'
--- lib/lp/translations/browser/productseries.py 2011-02-25 11:45:11 +0000
+++ lib/lp/translations/browser/productseries.py 2011-03-08 11:06:01 +0000
@@ -49,6 +49,9 @@
from lp.translations.browser.poexportrequest import BaseExportView
from lp.translations.browser.potemplate import BaseSeriesTemplatesView
from lp.translations.browser.translations import TranslationsMixin
+from lp.translations.browser.translationsharing import (
+ TranslationSharingDetailsMixin,
+ )
from lp.translations.interfaces.productserieslanguage import (
IProductSeriesLanguageSet,
)
@@ -344,7 +347,9 @@
return check_permission("launchpad.Edit", self.context)
-class ProductSeriesView(LaunchpadView, ProductSeriesTranslationsMixin):
+class ProductSeriesView(LaunchpadView,
+ ProductSeriesTranslationsMixin,
+ TranslationSharingDetailsMixin):
"""A view to show a series with translations."""
label = "Translation status by language"
@@ -462,6 +467,12 @@
sourcepackage, template = infos[0]
return sourcepackage
+ def getTranslationTarget(self):
+ """See `TranslationSharingDetailsMixin`."""
+ return self.context
+
+ can_edit_sharing_details = can_configure_translations
+
class SettingsRadioWidget(LaunchpadRadioWidgetWithDescription):
"""Remove the confusing hint under the widget."""
=== modified file 'lib/lp/translations/browser/sourcepackage.py'
--- lib/lp/translations/browser/sourcepackage.py 2011-02-25 11:45:11 +0000
+++ lib/lp/translations/browser/sourcepackage.py 2011-03-08 11:06:01 +0000
@@ -16,16 +16,21 @@
Link,
NavigationMenu,
)
+from canonical.launchpad.webapp.authorization import check_permission
from lp.registry.interfaces.sourcepackage import ISourcePackage
from lp.translations.browser.poexportrequest import BaseExportView
from lp.translations.browser.translations import TranslationsMixin
+from lp.translations.browser.translationsharing import (
+ TranslationSharingDetailsMixin,
+ )
from lp.translations.utilities.translationsharinginfo import (
has_upstream_template,
get_upstream_sharing_info,
)
-class SourcePackageTranslationsView(TranslationsMixin):
+class SourcePackageTranslationsView(TranslationsMixin,
+ TranslationSharingDetailsMixin):
@property
def potemplates(self):
@@ -47,6 +52,13 @@
productseries, template = infos[0]
return productseries
+ def getTranslationTarget(self):
+ """See `TranslationSharingDetailsMixin`."""
+ return self.context
+
+ def can_edit_sharing_details(self):
+ return check_permission('launchpad.Edit', self.context.distroseries)
+
class SourcePackageTranslationsMenu(NavigationMenu):
usedfor = ISourcePackage
=== modified file 'lib/lp/translations/browser/tests/test_sharing_information.py'
--- lib/lp/translations/browser/tests/test_sharing_information.py 2011-03-01 20:05:22 +0000
+++ lib/lp/translations/browser/tests/test_sharing_information.py 2011-03-08 11:06:01 +0000
@@ -62,12 +62,28 @@
SHARING_TEXT = None
- def _test_sharing_information(self, obj, expected_text):
+ def getAuthorizedUser(self, obj):
+ """Get a user that is authorized to edit sharing details on obj."""
+ raise NotImplementedError
+
+ SHARING_DETAILS_INFO = "View sharing details"
+ SHARING_DETAILS_SETUP = "Set up sharing"
+ SHARING_DETAILS_EDIT = "Edit sharing details"
+
+ def _test_sharing_information(self, obj,
+ id_under_test, expected_text,
+ authorized=False):
self.useFixture(FeatureFixture(
{'translations.sharing_information.enabled': 'on'}))
+ if authorized:
+ user = self.getAuthorizedUser(obj)
+ else:
+ user = None
browser = self.getViewBrowser(
- obj, no_login=True, rootsite="translations")
- sharing_info = find_tag_by_id(browser.contents, 'sharing-information')
+ obj, user=user, no_login=(not authorized),
+ rootsite="translations")
+
+ sharing_info = find_tag_by_id(browser.contents, id_under_test)
if expected_text is None:
self.assertIs(None, sharing_info)
else:
@@ -77,11 +93,36 @@
def test_not_sharing(self):
self._test_sharing_information(
- self.makeNotSharingObject(), self.NOT_SHARING_TEXT)
+ self.makeNotSharingObject(),
+ 'sharing-information', self.NOT_SHARING_TEXT)
def test_sharing(self):
self._test_sharing_information(
- self.makeSharingObject(), self.SHARING_TEXT)
+ self.makeSharingObject(),
+ 'sharing-information', self.SHARING_TEXT)
+
+ def test_sharing_details_info(self):
+ # For unauthorized users, the link to the sharing details page is
+ # informational.
+ self._test_sharing_information(
+ self.makeSharingObject(),
+ 'sharing-details', self.SHARING_DETAILS_INFO)
+
+ def test_sharing_details_setup(self):
+ # For authorized users of not sharing objects, the link to the
+ # sharing details page encourages action.
+ self._test_sharing_information(
+ self.makeNotSharingObject(),
+ 'sharing-details', self.SHARING_DETAILS_SETUP,
+ authorized=True)
+
+ def test_sharing_details_edit(self):
+ # For authorized users, the link to the sharing details page is for
+ # editing
+ self._test_sharing_information(
+ self.makeSharingObject(),
+ 'sharing-details', self.SHARING_DETAILS_EDIT,
+ authorized=True)
class TestUpstreamPOTemplateSharingInfo(BrowserTestCase,
@@ -104,6 +145,9 @@
SHARING_TEXT = """
This template is sharing translations with .*"""
+ def getAuthorizedUser(self, potemplate):
+ return potemplate.productseries.product.owner
+
class TestPOFileSharingInfo(BrowserTestCase, TestSharingInfoMixin):
"""Test display of POFile sharing info."""
@@ -125,6 +169,13 @@
SHARING_TEXT = """
These translations are shared with .*"""
+ def getAuthorizedUser(self, productseries):
+ return None
+
+ SHARING_DETAILS_INFO = None
+ SHARING_DETAILS_SETUP = None
+ SHARING_DETAILS_EDIT = None
+
class TestDummyPOFileSharingInfo(BrowserTestCase, TestSharingInfoMixin):
"""Test display of DummyPOFile sharing info."""
@@ -145,6 +196,13 @@
SHARING_TEXT = """
These translations are shared with .*"""
+ def getAuthorizedUser(self, productseries):
+ return None
+
+ SHARING_DETAILS_INFO = None
+ SHARING_DETAILS_SETUP = None
+ SHARING_DETAILS_EDIT = None
+
class TestUpstreamSharingInfo(BrowserTestCase, TestSharingInfoMixin):
"""Test display of product series sharing info."""
@@ -167,6 +225,9 @@
SHARING_TEXT = """
This project series is sharing translations with .*"""
+ def getAuthorizedUser(self, productseries):
+ return productseries.product.owner
+
class TestUbuntuPOTemplateSharingInfo(BrowserTestCase, TestSharingInfoMixin):
"""Test display of template sharing info in an Ubuntu source package."""
@@ -190,6 +251,12 @@
SHARING_TEXT = """
This template is sharing translations with .*"""
+ def getAuthorizedUser(self, potemplate):
+ with celebrity_logged_in('admin'):
+ potemplate.distroseries.owner = self.factory.makePerson(
+ password='test')
+ return potemplate.distroseries.owner
+
class TestUbuntuSharingInfo(BrowserTestCase, TestSharingInfoMixin):
"""Test display of source package sharing info."""
@@ -213,3 +280,9 @@
SHARING_TEXT = """
This source package is sharing translations with .*"""
+
+ def getAuthorizedUser(self, sourcepackage):
+ with celebrity_logged_in('admin'):
+ sourcepackage.distroseries.owner = self.factory.makePerson(
+ password='test')
+ return sourcepackage.distroseries.owner
=== added file 'lib/lp/translations/browser/translationsharing.py'
--- lib/lp/translations/browser/translationsharing.py 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/browser/translationsharing.py 2011-03-08 11:06:01 +0000
@@ -0,0 +1,49 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Views and mixins to use for translation sharing."""
+
+__metaclass__ = type
+
+__all__ = [
+ 'TranslationSharingDetailsMixin',
+ ]
+
+
+from canonical.launchpad.webapp import canonical_url
+
+
+class TranslationSharingDetailsMixin:
+ """Mixin for views that need to display translation details link.
+
+ View using this need to implement is_sharing, can_edit_sharing_details
+ and getTranslationTarget().
+ """
+
+ def getTranslationTarget(self):
+ """Return either a productseries or a sourcepackage."""
+ raise NotImplementedError
+
+ def sharing_details(self):
+ """Construct the link to the sharing details page."""
+ tag_template = (
+ '<a class="sprite %(icon)s" id="sharing-details"'
+ ' href="%(href)s/+sharing-details">%(text)s</a>')
+
+ if self.can_edit_sharing_details():
+ icon = 'edit'
+ if self.is_sharing():
+ text = "Edit sharing details"
+ else:
+ text = "Set up sharing"
+ else:
+ if self.is_sharing():
+ icon = 'info'
+ text = "View sharing details"
+ else:
+ return ""
+ href = canonical_url(
+ self.getTranslationTarget(),
+ rootsite='translations',
+ )
+ return tag_template % dict(icon=icon, text=text, href=href)
=== modified file 'lib/lp/translations/stories/standalone/xx-potemplate-index.txt'
--- lib/lp/translations/stories/standalone/xx-potemplate-index.txt 2011-03-02 09:20:38 +0000
+++ lib/lp/translations/stories/standalone/xx-potemplate-index.txt 2011-03-08 11:06:01 +0000
@@ -80,6 +80,7 @@
Sharing Information
This template is sharing translations with
evolution in Ubuntu Hoary template evolution-2.2.
+ View sharing details
>>> print sharing_info
<div...<a href="/ubuntu/hoary/+source/evolution/+pots/evolution-2.2"...
@@ -95,9 +96,23 @@
Sharing Information
This template is sharing translations with
Evolution trunk series template evolution-2.2.
+ View sharing details
>>> print sharing_info
<div...<a href="/evolution/trunk/+pots/evolution-2.2"...
+If the user has the right permissions, they are offered to edit the sharing
+information.
+
+ >>> with FeatureFixture(feature_flag):
+ ... admin_browser.open('http://translations.launchpad.dev/evolution'
+ ... '/trunk/+pots/evolution-2.2')
+ >>> sharing_details = find_tag_by_id(
+ ... admin_browser.contents, 'sharing-details')
+ >>> print extract_text(sharing_details)
+ Edit sharing details
+ >>> print sharing_details['href']
+ http://.../evolution/trunk/+sharing-details
+
Finding related templates
-------------------------
=== modified file 'lib/lp/translations/templates/potemplate-index.pt'
--- lib/lp/translations/templates/potemplate-index.pt 2011-03-01 17:39:16 +0000
+++ lib/lp/translations/templates/potemplate-index.pt 2011-03-08 11:06:01 +0000
@@ -80,6 +80,9 @@
tal:content="template/name"
>evolution-2.2</a>.
</p>
+ <a tal:replace="structure view/sharing_details">
+ View sharing details
+ </a>
</div>
</div>
=== modified file 'lib/lp/translations/templates/productseries-translations.pt'
--- lib/lp/translations/templates/productseries-translations.pt 2011-02-28 17:06:03 +0000
+++ lib/lp/translations/templates/productseries-translations.pt 2011-03-08 11:06:01 +0000
@@ -1,3 +1,4 @@
+
<html
xmlns="http://www.w3.org/1999/xhtml";
xmlns:tal="http://xml.zope.org/namespaces/tal";
@@ -90,6 +91,9 @@
tal:attributes="href package/fmt:url"
tal:content="package/displayname">apache in ubuntu hoary</a>.
</p>
+ <a tal:replace="structure view/sharing_details">
+ View sharing details
+ </a>
</div>
</div>
=== modified file 'lib/lp/translations/templates/sourcepackage-translations.pt'
--- lib/lp/translations/templates/sourcepackage-translations.pt 2011-02-28 17:06:03 +0000
+++ lib/lp/translations/templates/sourcepackage-translations.pt 2011-03-08 11:06:01 +0000
@@ -38,6 +38,9 @@
tal:attributes="href productseries/fmt:url"
tal:content="productseries/title">Evolution trunk</a>.
</p>
+ <a tal:replace="structure view/sharing_details">
+ View sharing details
+ </a>
</div>
</div>
<div class="yui-u">
