launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02250
[Merge] lp:~henninge/launchpad/db-devel-694703-partial-export into lp:launchpad/db-devel
Henning Eggers has proposed merging lp:~henninge/launchpad/db-devel-694703-partial-export into lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#694703 Make partial translations exports work for upstream
https://bugs.launchpad.net/bugs/694703
For more details, see:
https://code.launchpad.net/~henninge/launchpad/db-devel-694703-partial-export/+merge/44795
= Summary =
The export form for POFiles has an option to only export strings that
were changed in Launchpad. These are now determined by looking at
Ubuntu translations and see where they differ from upstream. This can
only be done when there is a packaging link between a project and a
source package.
Also, we only ever want to see translations chanaged on the Ubuntu
side, even if the export is requested from the upstream side.
== Proposed fix ==
Add a property on the POFile model that figures out the "other side
pofile".
Let the view request an export of that "other side pofile" if a
partial export is requested on an upstream pofile.
Add a view property to indicate if the option is to be displayed at
all. That property reflects if an "other side pofile" exists or not.
Use it in TAL to hide the option.
== Pre-implementation notes ==
Danilo and I had an actual voice call about this solution. ;)
== Implementation details ==
I changed BaseExportView.modifyFormat to directly get the information
from the form. That's all the call site was doing anyway and this way
I can use it in processForm without passing a parameter. Consequently
renamed it to getExportFormat.
When reading the tests please be aware that creating a POFile also
creates corresponding POFiles in sharing templates, just as creating
a new template will initialize it with the pofiles from sharing
templates. Since creating the packaging link possibly adds more
sharing templates to the pool, its placement has bearing on how the
test runs.
I had first thought to use a POTemplateSharingSubset because it
already implements similar logic to what is needed for the
other_side_pofile property. But it is too general because I already
know a specific series on one side, no need to expand that to a full
product or distribution. Still it might be a good idea to put these
side relations into a common class somehow.
== Tests ==
bin/test -vvcm lp.translations -t pofile -t export
That should about cover it. ;)
== Demo and Q/A ==
https://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/+export
Under this you'll find an export page where the option is hidden.
https://translations.launchpad.dev/alsa-utils/trunk/
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/stories/standalone/xx-pofile-export.txt
lib/lp/translations/tests/test_pofile.py
lib/lp/translations/browser/tests/test_poexportrequest_views.py
lib/lp/translations/interfaces/pofile.py
lib/lp/translations/templates/pofile-export.pt
lib/lp/translations/browser/poexportrequest.py
lib/lp/translations/browser/pofile.py
lib/lp/translations/model/pofile.py
./lib/lp/translations/browser/pofile.py
788: E301 expected 1 blank line, found 2
903: E301 expected 1 blank line, found 2
--
https://code.launchpad.net/~henninge/launchpad/db-devel-694703-partial-export/+merge/44795
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/db-devel-694703-partial-export into lp:launchpad/db-devel.
=== modified file 'lib/lp/translations/browser/poexportrequest.py'
--- lib/lp/translations/browser/poexportrequest.py 2010-11-15 16:25:05 +0000
+++ lib/lp/translations/browser/poexportrequest.py 2010-12-28 17:39:57 +0000
@@ -111,13 +111,9 @@
pofiles_ids = None
return (translation_templates_ids, pofiles_ids)
- def modifyFormat(self, format):
- """Optional overridable: return format used to export `format` files.
-
- :param format: What file format to look up an exportable format for.
- :returns: The modified format.
- """
- return format
+ def getExportFormat(self):
+ """Optional overridable: The requested export format."""
+ return self.request.form.get("format")
def initialize(self):
self.request_set = getUtility(IPOExportRequestSet)
@@ -131,7 +127,7 @@
return
bad_format_message = _("Please select a valid format for download.")
- format_name = self.modifyFormat(self.request.form.get("format"))
+ format_name = self.getExportFormat()
if format_name is None:
self.request.response.addErrorNotification(bad_format_message)
return
=== modified file 'lib/lp/translations/browser/pofile.py'
--- lib/lp/translations/browser/pofile.py 2010-12-09 19:49:36 +0000
+++ lib/lp/translations/browser/pofile.py 2010-12-28 17:39:57 +0000
@@ -51,6 +51,7 @@
CurrentTranslationMessageView,
)
from lp.translations.interfaces.pofile import IPOFile
+from lp.translations.interfaces.side import TranslationSide
from lp.translations.interfaces.translationimporter import (
ITranslationImporter,
)
@@ -989,19 +990,31 @@
page_title = "Download translation"
- def modifyFormat(self, format):
+ def getExportFormat(self):
+ format = self.request.form.get("format")
pochanged = self.request.form.get("pochanged")
if format == 'PO' and pochanged == 'POCHANGED':
return 'POCHANGED'
return format
def processForm(self):
+ is_upstream = (
+ self.context.potemplate.translation_side ==
+ TranslationSide.UPSTREAM)
+ if is_upstream and self.getExportFormat() == 'POCHANGED':
+ if self.context.other_side_pofile is None:
+ return None
+ return (None, [self.context.other_side_pofile])
return (None, [self.context])
def getDefaultFormat(self):
return self.context.potemplate.source_file_format
@property
+ def show_pochanged_option(self):
+ return self.context.other_side_pofile is not None
+
+ @property
def cancel_url(self):
return canonical_url(self.context)
=== modified file 'lib/lp/translations/browser/tests/test_poexportrequest_views.py'
--- lib/lp/translations/browser/tests/test_poexportrequest_views.py 2010-12-27 15:35:17 +0000
+++ lib/lp/translations/browser/tests/test_poexportrequest_views.py 2010-12-28 17:39:57 +0000
@@ -3,6 +3,8 @@
__metaclass__ = type
+from zope.security.proxy import removeSecurityProxy
+
from canonical.launchpad.interfaces.lpstorm import IStore
from canonical.launchpad.webapp.servers import LaunchpadTestRequest
from canonical.testing.layers import DatabaseFunctionalLayer
@@ -128,9 +130,12 @@
# All exports can be requested by an unprivileged user.
self.translator = self.factory.makePerson()
- def _createView(self, form):
+ def _createView(self, form=None):
login_person(self.translator)
- request = LaunchpadTestRequest(method='POST', form=form)
+ if form is None:
+ request = LaunchpadTestRequest()
+ else:
+ request = LaunchpadTestRequest(method='POST', form=form)
view = POExportView(self.pofile, request)
view.initialize()
return view
@@ -139,7 +144,7 @@
# It is possible to request an export in the PO format.
self._createView({'format': 'PO'})
- self.assertContentEqual(
+ self.assertEqual(
[(self.potemplate, self.pofile, TranslationFileFormat.PO)],
get_poexportrequests(include_format=True))
@@ -147,22 +152,87 @@
# It is possible to request an export in the MO format.
self._createView({'format': 'MO'})
- self.assertContentEqual(
+ self.assertEqual(
[(self.potemplate, self.pofile, TranslationFileFormat.MO)],
get_poexportrequests(include_format=True))
- def test_request_partial_po(self):
- # Partial po exports are requested by an extra check box.
- self._createView({'format': 'PO', 'pochanged': 'POCHANGED'})
-
- self.assertContentEqual(
+ def _makeUbuntuTemplateAndPOFile(self):
+ """Create a sharing template in Ubuntu with a POFile to go with it."""
+ upstream_series = self.potemplate.productseries
+ template_name = self.potemplate.name
+ language = self.pofile.language
+
+ distroseries = self.factory.makeUbuntuDistroSeries()
+ naked_distribution = removeSecurityProxy(distroseries.distribution)
+ naked_distribution.translation_focus = distroseries
+ sourcepackagename = self.factory.makeSourcePackageName()
+ sourcepackage = self.factory.makeSourcePackage(
+ sourcepackagename, distroseries)
+ sourcepackage.setPackaging(upstream_series, self.factory.makePerson())
+
+ potemplate = self.factory.makePOTemplate(
+ distroseries=distroseries, sourcepackagename=sourcepackagename,
+ name=template_name)
+
+ # The sharing POFile is created automatically when the template is
+ # created because self.pofile already exists.
+ return (potemplate, potemplate.getPOFileByLang(language.code))
+
+ def test_request_partial_po_upstream(self):
+ # Partial po exports are requested by an extra check box.
+ # For an upstream project, the export is requested on the
+ # corresponding (same language, sharing template) on the Ubuntu side.
+ ubuntu_potemplate, ubuntu_pofile = self._makeUbuntuTemplateAndPOFile()
+
+ self._createView({'format': 'PO', 'pochanged': 'POCHANGED'})
+
+ expected = (
+ ubuntu_potemplate,
+ ubuntu_pofile,
+ TranslationFileFormat.POCHANGED,
+ )
+ self.assertEqual(
+ [expected], get_poexportrequests(include_format=True))
+
+ def test_request_partial_po_ubuntu(self):
+ # Partial po exports are requested by an extra check box.
+ # For an Ubuntu package, the export is requested on the file itself.
+ ubuntu_potemplate, ubuntu_pofile = self._makeUbuntuTemplateAndPOFile()
+ self.potemplate = ubuntu_potemplate
+ self.pofile = ubuntu_pofile
+
+ self._createView({'format': 'PO', 'pochanged': 'POCHANGED'})
+
+ self.assertEqual(
[(self.potemplate, self.pofile, TranslationFileFormat.POCHANGED)],
get_poexportrequests(include_format=True))
+ def test_request_partial_po_no_link(self):
+ # Partial po exports are requested by an extra check box.
+ # Without a packaging link, no request is created.
+ self._createView({'format': 'PO', 'pochanged': 'POCHANGED'})
+
+ self.assertEqual([], get_poexportrequests())
+
def test_request_partial_mo(self):
# With the MO format, the partial export check box is ignored.
self._createView({'format': 'MO', 'pochanged': 'POCHANGED'})
- self.assertContentEqual(
+ self.assertEqual(
[(self.potemplate, self.pofile, TranslationFileFormat.MO)],
get_poexportrequests(include_format=True))
+
+ def test_pochanged_option_available(self):
+ # The option for partial exports is only available if a POFile can
+ # be found on the other side.
+ self._makeUbuntuTemplateAndPOFile()
+ view = self._createView()
+
+ self.assertTrue(view.show_pochanged_option)
+
+ def test_pochanged_option_not_available(self):
+ # The option for partial exports is not available if no POFile can
+ # be found on the other side.
+ view = self._createView()
+
+ self.assertFalse(view.show_pochanged_option)
=== modified file 'lib/lp/translations/interfaces/pofile.py'
--- lib/lp/translations/interfaces/pofile.py 2010-12-09 19:49:36 +0000
+++ lib/lp/translations/interfaces/pofile.py 2010-12-28 17:39:57 +0000
@@ -153,6 +153,10 @@
'''),
required=True, readonly=True)
+ other_side_pofile = Attribute(_(
+ "The POFile for the same language in the sharing template on the "
+ "other side of a packaging link. None if no link exists."))
+
def translatedCount():
"""
Returns the number of message sets which this PO file has current
=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py 2010-12-09 19:49:36 +0000
+++ lib/lp/translations/model/pofile.py 2010-12-28 17:39:57 +0000
@@ -66,6 +66,7 @@
MASTER_FLAVOR,
)
from canonical.launchpad.webapp.publisher import canonical_url
+from lp.app.errors import NotFoundError
from lp.registry.interfaces.person import validate_public_person
from lp.services.propertycache import cachedproperty
from lp.translations.enums import RosettaImportStatus
@@ -73,6 +74,9 @@
IPOFile,
IPOFileSet,
)
+from lp.translations.interfaces.potemplate import (
+ IPOTemplateSet,
+ )
from lp.translations.interfaces.potmsgset import (
BrokenTextError,
TranslationCreditsType,
@@ -388,6 +392,44 @@
"""See `IPOFile`."""
return self.getTranslationMessages()
+ def _getOrCreateMatchingPOFile(self, other_potemplate):
+ other_pofile = other_potemplate.getPOFileByLang(
+ self.language.code)
+ if other_pofile is None:
+ other_pofile = other_potemplate.newPOFile(self.language.code)
+ return other_pofile
+
+ @cachedproperty
+ def other_side_pofile(self):
+ """See `IPOFile`."""
+ potemplateset = getUtility(IPOTemplateSet)
+ if self.potemplate.translation_side == TranslationSide.UBUNTU:
+ from lp.registry.model.sourcepackage import SourcePackage
+ productseries = SourcePackage(
+ self.potemplate.sourcepackagename,
+ self.potemplate.distroseries).productseries
+ if productseries is None:
+ return None
+ subset = potemplateset.getSubset(productseries=productseries)
+ else:
+ ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+ distroseries = ubuntu.translation_focus
+ if distroseries is None:
+ distroseries = ubuntu.currentseries
+ try:
+ sourcepackage = self.potemplate.productseries.getPackage(
+ distroseries)
+ except NotFoundError:
+ return None
+ subset = potemplateset.getSubset(
+ distroseries=distroseries,
+ sourcepackagename=sourcepackage.sourcepackagename)
+
+ other_potemplate = subset.getPOTemplateByName(self.potemplate.name)
+ if other_potemplate is None:
+ return None
+ return self._getOrCreateMatchingPOFile(other_potemplate)
+
def getTranslationMessages(self, condition=None):
"""See `IPOFile`."""
applicable_template = Coalesce(
@@ -649,7 +691,7 @@
clauses.extend([
'TranslationTemplateItem.potmsgset = POTMsgSet.id',
'TranslationMessage.%s IS NOT TRUE' % flag_name,
- "(%s)" % msgstr_clause
+ "(%s)" % msgstr_clause,
])
diverged_translation_query = (
@@ -699,7 +741,7 @@
"""See `IPOFile`."""
# A `POTMsgSet` has different translations if both sides have a
# translation. If one of them is empty, the POTMsgSet is not included
- # in this list.
+ # in this list.
clauses, clause_tables = self._getTranslatedMessagesQuery()
other_side_flag_name = getUtility(
@@ -722,7 +764,7 @@
dict(
flag_name=other_side_flag_name,
potemplate=quote(self.potemplate),
- language=quote(self.language))
+ language=quote(self.language)),
))
imported_clauses = [
'imported.id <> TranslationMessage.id',
@@ -1274,6 +1316,7 @@
self.contributors = []
self.from_sourcepackagename = None
self.translation_messages = None
+ self.other_side_pofile = None
def messageCount(self):
return self.potemplate.messageCount()
=== modified file 'lib/lp/translations/templates/pofile-export.pt'
--- lib/lp/translations/templates/pofile-export.pt 2009-12-03 18:33:22 +0000
+++ lib/lp/translations/templates/pofile-export.pt 2010-12-28 17:39:57 +0000
@@ -60,7 +60,8 @@
>PO format</option>
</select>
</div>
- <div id="div_pochanged" style="margin-top: 10px">
+ <div id="div_pochanged" style="margin-top: 10px"
+ tal:condition="view/show_pochanged_option">
<input type="checkbox" name="pochanged" id="cb_pochanged"
value="POCHANGED" />
<label for="cb_pochanged">
=== modified file 'lib/lp/translations/tests/test_pofile.py'
--- lib/lp/translations/tests/test_pofile.py 2010-12-09 19:49:36 +0000
+++ lib/lp/translations/tests/test_pofile.py 2010-12-28 17:39:57 +0000
@@ -1951,6 +1951,133 @@
self.assertTrue(pofile.hasPluralFormInformation())
+class TestPOFileUbuntuUpstreamSharingMixin:
+ """Test sharing between Ubuntu und upstream POFiles."""
+
+ layer = ZopelessDatabaseLayer
+
+ def createData(self):
+ self.shared_template_name = self.factory.getUniqueString()
+ self.shared_language = self.factory.makeLanguage()
+
+ self.distroseries = self.factory.makeUbuntuDistroSeries()
+ self.distroseries.distribution.translation_focus = (
+ self.distroseries)
+ self.sourcepackagename = self.factory.makeSourcePackageName()
+ self.sourcepackage = self.factory.makeSourcePackage(
+ distroseries=self.distroseries,
+ sourcepackagename=self.sourcepackagename)
+ self.productseries = self.factory.makeProductSeries()
+
+ def makeThisSidePOFile(self):
+ """Create POFile on this side. Override in subclass."""
+ raise NotImplementedError
+
+ def makeOtherSidePOFile(self):
+ """Create POFile on the other side. Override in subclass."""
+ raise NotImplementedError
+
+ def makeOtherSidePOTemplate(self):
+ """Create POTemplate on the other side. Override in subclass."""
+ raise NotImplementedError
+
+ def makeSharedUbuntuPOTemplate(self):
+ """Create template ready for sharing on the Ubuntu side."""
+ return self.factory.makePOTemplate(
+ distroseries=self.distroseries,
+ sourcepackagename=self.sourcepackagename,
+ name=self.shared_template_name)
+
+ def makeSharedUbuntuPOFile(self):
+ """Create template and POFile ready for sharing on the Ubuntu side.
+ """
+ potemplate = self.makeSharedUbuntuPOTemplate()
+ return self.factory.makePOFile(
+ language=self.shared_language, potemplate=potemplate)
+
+ def makeSharedUpstreamPOTemplate(self):
+ """Create template ready for sharing on the upstream side."""
+ return self.factory.makePOTemplate(
+ productseries=self.productseries,
+ name=self.shared_template_name)
+
+ def makeSharedUpstreamPOFile(self):
+ """Create template and POFile ready for sharing on the upstream side.
+ """
+ potemplate = self.makeSharedUpstreamPOTemplate()
+ return self.factory.makePOFile(
+ language=self.shared_language, potemplate=potemplate)
+
+ def _setPackagingLink(self):
+ """Create the packaging link from source package to product series."""
+ # Packaging links want an owner.
+ self.sourcepackage.setPackaging(
+ self.productseries, self.factory.makePerson())
+
+ def test_other_side_pofile_none(self):
+ # Without a packaging link, None is returned.
+ pofile = self.makeThisSidePOFile()
+ self.assertIs(None, pofile.other_side_pofile)
+
+ def test_other_side_pofile_linked_no_template(self):
+ # If no sharing template exists on the other side, no POFile can be
+ # found, even with a packaging link.
+ self._setPackagingLink()
+ pofile = self.makeThisSidePOFile()
+ self.assertIs(None, pofile.other_side_pofile)
+
+ def test_other_side_pofile_shared(self):
+ # This is how sharing should look like.
+ this_pofile = self.makeThisSidePOFile()
+ other_pofile = self.makeOtherSidePOFile()
+ self._setPackagingLink()
+ self.assertEquals(other_pofile, this_pofile.other_side_pofile)
+
+ def test_other_side_pofile_missing(self):
+ # If the other side pofile is missing, it is created automatically.
+ this_pofile = self.makeThisSidePOFile()
+ other_potemplate = self.makeOtherSidePOTemplate()
+ self._setPackagingLink()
+ self.assertEquals(
+ other_potemplate, this_pofile.other_side_pofile.potemplate)
+
+
+class TestPOFileUbuntuSharing(TestCaseWithFactory,
+ TestPOFileUbuntuUpstreamSharingMixin):
+ """Test sharing on Ubuntu side."""
+
+ def setUp(self):
+ super(TestPOFileUbuntuSharing, self).setUp()
+ self.createData()
+
+ def makeThisSidePOFile(self):
+ return self.makeSharedUbuntuPOFile()
+
+ def makeOtherSidePOFile(self):
+ return self.makeSharedUpstreamPOFile()
+
+ def makeOtherSidePOTemplate(self):
+ return self.makeSharedUpstreamPOTemplate()
+
+
+class TestPOFileUpstreamSharing(TestCaseWithFactory,
+ TestPOFileUbuntuUpstreamSharingMixin):
+ """Test sharing on upstream side."""
+
+ def setUp(self):
+ super(TestPOFileUpstreamSharing, self).setUp()
+ self.createData()
+
+ def makeThisSidePOFile(self):
+ return self.makeSharedUpstreamPOFile()
+
+ def makeOtherSidePOFile(self):
+ return self.makeSharedUbuntuPOFile()
+
+ def makeOtherSidePOTemplate(self):
+ return self.makeSharedUbuntuPOTemplate()
+
+
class TestPOFileTranslationMessages(TestCaseWithFactory):
"""Test PO file getTranslationMessages method."""
@@ -2842,4 +2969,3 @@
potemplate=self.factory.makePOTemplate(
distroseries=package.distroseries,
sourcepackagename=package.sourcepackagename))
-
