Merge lp://staging/~jtv/launchpad/recife-add-suggestion into lp://staging/~launchpad/launchpad/recife

> === modified file 'lib/lp/translations/model/potmsgset.py'
> --- lib/lp/translations/model/potmsgset.py 2010-03-23 15:39:12
+0000
> +++ lib/lp/translations/model/potmsgset.py 2010-03-25 00:11:17
+0000
> @@ -888,6 +888,36 @@
> matching_message.sync()
> return matching_message
>
> + def submitSuggestion(self, pofile, submitter, new_translations):
> + """See `IPOTMsgSet`."""
> + if self.is_translation_credit:
> + # We don't support suggestions on credits messages.
> + return None
> +
> + potranslations = self._findPOTranslations(new_translations)
> +
> + existing_message = self._findTranslationMessage(
> + pofile, potranslations, pofile.plural_forms)
> + if existing_message is not None:
> + return existing_message

In the end you still included this logic as part of submitSuggestion? I
guess we can move it out later if it turns out that's what we want.

> === modified file 'lib/lp/translations/tests/test_potmsgset.py'
> @@ -1267,5 +1271,274 @@
> self.assertEqual(credits_type,
credits.translation_credits_type)
>
>
> +class KarmaListener:

Any reason not to put this inside lib/lp/registry/tests/karma.py?

> +class TestPOTMsgSet_submitSuggestion(TestCaseWithFactory):
> + """Test `POTMsgSet.submitSuggestion`."""
> +
> + layer = ZopelessDatabaseLayer
> +
> + def _makePOFileAndPOTMsgSet(self, msgid=None, with_plural=False):

In general, if you find that you need such a method, we might probably
need it in other cases as well. So, why not put it into the factory?

> + """Set up a `POFile` with `POTMsgSet`."""
> + pofile = self.factory.makePOFile('nl')
> + if msgid is None:
> + singular_text = self.factory.getUniqueString()
> + else:
> + singular_text = msgid
> + if with_plural:
> + plural_text = self.factory.getUniqueString()
> + else:
> + plural_text = None
> + potmsgset = self.factory.makePOTMsgSet(
> + pofile.potemplate, singular=singular_text,
plural=plural_text)
> +

Some extra whitespace here.

> + return pofile, potmsgset
> +
> + def _suggest(self, pofile, potmsgset, translations):
> + """Convenience shorthand for submitSuggestion."""
> + return potmsgset.submitSuggestion(
> + pofile, pofile.potemplate.owner, translations)

I believe this reduced the usefulness of these tests as a documentation
for submitSuggestion() method. And yet, you have added no documentation
whatsoever.

> + def test_plural_forms(self):
> + pofile, potmsgset =
self._makePOFileAndPOTMsgSet(with_plural=True)
> + translations = {
> + 0: self.factory.getUniqueString(),
> + 1: self.factory.getUniqueString(),
> + }
> +
> + suggestion = self._suggest(pofile, potmsgset, translations)
> + for form, translation in translations.iteritems():
> + self.assertEqual(
> + translation,
> + getattr(suggestion, 'msgstr%d' % form).translation)
> + for form in xrange(2, TranslationConstants.MAX_PLUR...

See revision 9132 above:
 * Renamed lp.registry.tests.karma to lp.testing.karma to deal with import problems.
 * Renamed KarmaListener to KarmaRecorder, moved into lp.testing.karma.
 * Made KarmaRecorder a bit more flexible, turned existing KarmaAssignedEventListener into a subclass.
 * Moved set_branding from canonical.launchpad.ftests to a new module lp.testing.branding to deal with import problems.
 * Moved makePOFileAndPOTMsgSet into the factory.
 * Removed test helper _suggest, calling submitSuggestion directly instead.
 * Used the factory to create TranslationMessages in the test, instead of updateTranslation.
 * Unrolled the poorly legible test loop. Don't really need to test _all_ possible plural forms.

Jeroen

All good.