launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01988
[Merge] lp:~jtv/launchpad/recife-pre-killUpdateTranslation-browser into lp:~launchpad/launchpad/recife
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/recife-pre-killUpdateTranslation-browser into lp:~launchpad/launchpad/recife.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
= Preparations for lp:~jtv/launchpad/recife-kill-updatetranslation-browser =
For the Recife feature branch. These are some cleanups that help simplify my main branch, which stops our browser code from using updateTranslation and moves a few views over to the new data model. Or some just fix up little annoyances along the way.
To walk you through the changes:
_storeTranslations is a fairly complicated method, made worse by the need to control all exit paths, turn exceptions into returned error strings, and call _observeTranslationUpdate in the right places. By introducing an intermediary called _receiveTranslations, I channel all exits from _storeTranslations through just a single "try" block.
This does give up a bit of detail in which error string is returned when. The old code used try/except to distinguish "your dismissing suggestions here conflicts with concurrent changes, so please review the result" versus "your translations here conflict with concurrent changes, so please review the result." The main difference is that the latter message also notes that the user's translations are not discarded, merely stored as suggestions. I don't think that saying that regardless of whether the user dismissed suggestions or entered suggestions will make a big difference. Especially since the latter message was also shown when the user merely cleared some existing translations.
I also rewrote the message a bit: "Somebody else changed this translation" is guessing at specifics. "This translation has changed" leaves out nothing important but is less likely to be wrong if, for instance, the same user was accidentally working from two browser windows on the same page.
For convenience in my later branch, I add a forwarding method to _validate_translations in POTMsgSet so I don't have to gather the same self.* values again.
Then, in lib/lp/translations/model/side.py, I eliminate a small piece of redundancy in our code. In lib/lp/translations/utilities/validate.py, I make some code I had to read easier to read.
To test, best run all Translations tests. The branch will be Q/A'ed with the rest of the Recife feature branch, very soon.
Jeroen
--
https://code.launchpad.net/~jtv/launchpad/recife-pre-killUpdateTranslation-browser/+merge/41465
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/recife-pre-killUpdateTranslation-browser into lp:~launchpad/launchpad/recife.
=== modified file 'lib/lp/translations/browser/pofile.py'
--- lib/lp/translations/browser/pofile.py 2010-11-18 05:28:57 +0000
+++ lib/lp/translations/browser/pofile.py 2010-11-22 14:19:59 +0000
@@ -835,7 +835,7 @@
"Got translation for POTMsgID %d which is not in the "
"template." % id)
- error = self._storeTranslations(potmsgset)
+ error = self._receiveTranslations(potmsgset)
if error and potmsgset.getSequence(self.context.potemplate) != 0:
# There is an error, we should store it to be rendered
# together with its respective view.
=== modified file 'lib/lp/translations/browser/tests/translationmessage-views.txt'
--- lib/lp/translations/browser/tests/translationmessage-views.txt 2010-11-18 05:28:57 +0000
+++ lib/lp/translations/browser/tests/translationmessage-views.txt 2010-11-22 14:19:59 +0000
@@ -291,9 +291,9 @@
There is an error in the translation you provided. Please correct it
before continuing.
>>> print translationmessage_page_view.error
- Somebody else changed this translation since you started. To avoid
+ This translation has changed since you last saw it. To avoid
accidentally reverting work done by others, we added your translations
- as suggestions, so please review current values.
+ as suggestions. Please review the current values.
>>> transaction.commit()
This submission is not saved because there is another modification, this
=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py 2010-11-04 09:32:28 +0000
+++ lib/lp/translations/browser/translationmessage.py 2010-11-22 14:19:59 +0000
@@ -377,7 +377,7 @@
Implementing this method is complicated. It needs to find out
what TranslationMessage were updated in the form post, call
- _storeTranslations() for each of those, check for errors that
+ _receiveTranslations() for each of those, check for errors that
may have occurred during that (displaying them using
addErrorNotification), and otherwise call _redirectToNextPage if
everything went fine.
@@ -389,29 +389,50 @@
# and _submitTranslations().
#
+ def _receiveTranslations(self, potmsgset):
+ """Process and store submitted translations for `potmsgset`.
+
+ :return: An error string in case of failure, or None otherwise.
+ """
+ try:
+ self._storeTranslations(potmsgset)
+ except GettextValidationError, e:
+ return unicode(e)
+ except TranslationConflict:
+ # The translations are demoted to suggestions, but they may
+ # still affect the "messages with new suggestions" filter.
+ self._observeTranslationUpdate(potmsgset)
+ return """
+ This translation has changed since you last saw it. To avoid
+ accidentally reverting work done by others, we added your
+ translations as suggestions. Please review the current
+ values.
+ """
+ else:
+ self._observeTranslationUpdate(potmsgset)
+ return None
+
def _storeTranslations(self, potmsgset):
"""Store the translation submitted for a POTMsgSet.
- Return a string with an error if one occurs, otherwise None.
+ :raises GettextValidationError: if the submitted translation
+ fails gettext validation. The translation is not stored.
+ :raises TranslationConflict: if the current translations have
+ changed since the translator/reviewer last saw them. The
+ submitted translations are stored as suggestions.
"""
self._extractFormPostedTranslations(potmsgset)
if self.form_posted_dismiss_suggestions.get(potmsgset, False):
- try:
- potmsgset.dismissAllSuggestions(self.pofile,
- self.user,
- self.lock_timestamp)
- except TranslationConflict, e:
- return unicode(e)
+ potmsgset.dismissAllSuggestions(
+ self.pofile, self.user, self.lock_timestamp)
return None
translations = self.form_posted_translations.get(potmsgset, {})
if not translations:
# A post with no content -- not an error, but nothing to be
# done.
- # XXX: kiko 2006-09-28: I'm not sure but I suspect this could
- # be an UnexpectedFormData.
- return None
+ return
plural_indices_to_store = (
self.form_posted_translations_has_store_flag.get(potmsgset, []))
@@ -438,42 +459,27 @@
if translationmessage is None and not has_translations:
# There is no current translation yet, neither we get any
# translation submitted, so we don't need to store anything.
- return None
+ return
force_suggestion = self.form_posted_needsreview.get(potmsgset, False)
force_diverge = self.form_posted_diverge.get(potmsgset, False)
- try:
- potmsgset.updateTranslation(
- self.pofile, self.user, translations,
- is_current_upstream=False,
- lock_timestamp=self.lock_timestamp,
- force_suggestion=force_suggestion,
- force_diverged=force_diverge)
-
- empty_suggestions = self._areSuggestionsEmpty(translations)
- if (force_suggestion and
- self.user_is_official_translator and
- empty_suggestions):
- # The user requested that the message be reviewed,
- # without suggesting a new translation. Reset the
- # current translation so that it can be reviewed again.
- potmsgset.old_resetCurrentTranslation(
- self.pofile, self.lock_timestamp)
-
- except TranslationConflict:
- return (
- u'Somebody else changed this translation since you started.'
- u' To avoid accidentally reverting work done by others, we'
- u' added your translations as suggestions, so please review'
- u' current values.')
- except GettextValidationError, e:
- # Save the error message gettext gave us to show it to the
- # user.
- return unicode(e)
- else:
- self._observeTranslationUpdate(potmsgset)
- return None
+ potmsgset.updateTranslation(
+ self.pofile, self.user, translations,
+ is_current_upstream=False,
+ lock_timestamp=self.lock_timestamp,
+ force_suggestion=force_suggestion,
+ force_diverged=force_diverge)
+
+ empty_suggestions = self._areSuggestionsEmpty(translations)
+ if (force_suggestion and
+ self.user_is_official_translator and
+ empty_suggestions):
+ # The user requested that the message be reviewed,
+ # without suggesting a new translation. Reset the
+ # current translation so that it can be reviewed again.
+ potmsgset.old_resetCurrentTranslation(
+ self.pofile, self.lock_timestamp)
def _areSuggestionsEmpty(self, suggestions):
"""Return true if all suggestions are empty strings or None."""
@@ -853,7 +859,7 @@
def _submitTranslations(self):
"""See `BaseTranslationView._submitTranslations`."""
- self.error = self._storeTranslations(self.context.potmsgset)
+ self.error = self._receiveTranslations(self.context.potmsgset)
if self.error:
self.request.response.addErrorNotification(
"There is an error in the translation you provided. "
=== modified file 'lib/lp/translations/interfaces/potmsgset.py'
--- lib/lp/translations/interfaces/potmsgset.py 2010-11-18 05:28:57 +0000
+++ lib/lp/translations/interfaces/potmsgset.py 2010-11-22 14:19:59 +0000
@@ -257,6 +257,15 @@
parameter set.
"""
+ def validateTranslations(translations):
+ """Validate `translations` against gettext.
+
+ :param translations: A dict mapping plural forms to translated
+ strings.
+ :raises GettextValidationError: if there is a problem with the
+ translations.
+ """
+
def submitSuggestion(pofile, submitter, new_translations):
"""Submit a suggested translation for this message.
=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py 2010-11-18 09:46:57 +0000
+++ lib/lp/translations/model/potmsgset.py 2010-11-22 14:19:59 +0000
@@ -532,6 +532,11 @@
return sanitized_translations
+ def validateTranslations(self, translations):
+ """See `IPOTMsgSet`."""
+ validate_translation(
+ self.singular_text, self.plural_text, translations, self.flags)
+
def _validate_translations(self, translations, ignore_errors):
"""Validate all the `translations` and return a validation_status."""
# By default all translations are correct.
@@ -540,9 +545,7 @@
# Validate the translation we got from the translation form
# to know if gettext is unhappy with the input.
try:
- validate_translation(
- self.singular_text, self.plural_text,
- translations, self.flags)
+ self.validateTranslations(translations)
except GettextValidationError:
if ignore_errors:
# The translations are stored anyway, but we set them as
=== modified file 'lib/lp/translations/model/side.py'
--- lib/lp/translations/model/side.py 2010-11-16 08:46:48 +0000
+++ lib/lp/translations/model/side.py 2010-11-22 14:19:59 +0000
@@ -65,11 +65,7 @@
def getForTemplate(self, potemplate):
"""See `ITranslationSideTraitsSet`."""
- if potemplate.productseries is not None:
- side = TranslationSide.UPSTREAM
- else:
- side = TranslationSide.UBUNTU
- return self.getTraits(side)
+ return self.getTraits(potemplate.translation_side)
def getAllTraits(self):
"""See `ITranslationSideTraitsSet`."""
=== modified file 'lib/lp/translations/stories/standalone/xx-translationmessage-translate.txt'
--- lib/lp/translations/stories/standalone/xx-translationmessage-translate.txt 2010-11-04 09:32:28 +0000
+++ lib/lp/translations/stories/standalone/xx-translationmessage-translate.txt 2010-11-22 14:19:59 +0000
@@ -430,9 +430,9 @@
<td>
</td><td>
<div>
- Somebody else changed this translation since you started. To avoid
+ This translation has changed since you last saw it. To avoid
accidentally reverting work done by others, we added your
- translations as suggestions, so please review current values.
+ translations as suggestions. Please review the current values.
</div>
</td>
</tr>
=== modified file 'lib/lp/translations/utilities/validate.py'
--- lib/lp/translations/utilities/validate.py 2010-09-28 14:06:50 +0000
+++ lib/lp/translations/utilities/validate.py 2010-11-22 14:19:59 +0000
@@ -31,15 +31,14 @@
msg = gettextpo.PoMessage()
msg.set_msgid(original_singular)
- if original_plural is not None:
- # It has plural forms.
+ if original_plural is None:
+ # Basic, single-form message.
+ msg.set_msgstr(translations.get(0))
+ else:
+ # Message with plural forms.
msg.set_msgid_plural(original_plural)
for form, translation in translations.iteritems():
msg.set_msgstr_plural(form, translation)
- elif 0 in translations:
- msg.set_msgstr(translations[0])
- else:
- pass
for flag in flags:
msg.set_format(flag, True)
