Heh... I enjoyed reading your MP description :)

And yes, I've been in that position before too... the one case (a transition to new infrastructure) where a bad name is a good one.

I've just got one question below about "the other side". But r=me. And great tests :)


> === modified file 'lib/lp/translations/browser/translationmessage.py'
> --- lib/lp/translations/browser/translationmessage.py	2010-08-23 08:35:29 +0000
> +++ lib/lp/translations/browser/translationmessage.py	2010-08-26 10:24:07 +0000
> @@ -300,16 +299,16 @@
>
>          if self.request.method == 'POST':
>              if self.user is None:
> -                raise UnexpectedFormData, (
> -                    'Anonymous users or users who are not accepting our '
> -                    'licensing terms cannot do POST submissions.')
> +                raise UnexpectedFormData(
> +                    "Anonymous users or users who are not accepting our "
> +                    "licensing terms cannot do POST submissions.")

Out of interest, is switching to double-quotes by default a personal
preference or a new LP coding recommendation?


> === modified file 'lib/lp/translations/model/potmsgset.py'
> --- lib/lp/translations/model/potmsgset.py	2010-08-24 11:39:06 +0000
> +++ lib/lp/translations/model/potmsgset.py	2010-08-26 10:24:07 +0000
> @@ -1295,25 +1295,47 @@
>
>          return message
>
> -    def resetCurrentTranslation(self, pofile, lock_timestamp):
> -        """See `IPOTMsgSet`."""
> -
> -        assert(lock_timestamp is not None)
> -
> +    def old_resetCurrentTranslation(self, pofile, lock_timestamp):
> +        """See `POTMsgSet`.
> +
> +        This message is OBSOLETE in the Recife feature branch.  It's
> +        still here only until we replace its one call with the new
> +        method.
> +        """
> +        assert lock_timestamp is not None, "No lock timestamp given."
>          current = self.getCurrentTranslationMessage(
>              pofile.potemplate, pofile.language)
> -
> -        if (current is not None):
> -            # Check for translation conflicts and update the required
> -            # attributes.
> -            self._maybeRaiseTranslationConflict(current, lock_timestamp)
> -            current.is_current_ubuntu = False
> -            # Converge the current translation only if it is diverged and not
> -            # current upstream.
> -            is_diverged = current.potemplate is not None
> -            if is_diverged and not current.is_current_upstream:
> -                current.potemplate = None
> -            pofile.date_changed = UTC_NOW
> +        if current is None:
> +            return

Nice... I prefer guard code like this too :)

> +
> +        # Check for translation conflicts and update the required
> +        # attributes.
> +        self._maybeRaiseTranslationConflict(current, lock_timestamp)
> +        current.is_current_ubuntu = False
> +        # Converge the current translation only if it is diverged and
> +        # not current upstream.
> +        if current.is_diverged and not current.is_current_upstream:
> +            current.potemplate = None
> +        pofile.markChanged()
> +
> +    def resetCurrentTranslation(self, pofile, lock_timestamp=None,
> +                                share_with_other_side=False):

OK, I forgot to mention this on the IFace declaration, but I have no idea what
the other side is (maybe the interface description could say a bit more to
cater for non-translation folk?). From your tests, it looks like it is
upstream/downstream?

> +        """See `IPOTMsgSet`."""
> +        traits = getUtility(ITranslationSideTraitsSet).getTraits(
> +            pofile.potemplate.translation_side)
> +        current_message = traits.getCurrentMessage(
> +            self, pofile.potemplate, pofile.language)
> +
> +        if current_message is None:
> +            # Nothing to do here.

Is this an error? If a reset is called on a translation that doesn't yet have
a translation? I'm sure you've thought of that and decided its not, but just in
case.