launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/translations-message-links-404 into lp:launchpad.

Commit message:
Blocks rendering of links to untraversable translation suggestions.

Requested reviews:
  Curtis Hovey (sinzui)
Related bugs:
  Bug #403629 in Launchpad itself: "Translation message link points to wrong message number"
  https://bugs.launchpad.net/launchpad/+bug/403629

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/translations-message-links-404/+merge/131052

Summary
=======
Translations creates links to translations suggestions, but some of these
suggestions aren't traversable because they are no longer contained properly
in a potmsgset (which creates an index of 0 for the message).

The solution is simply to add a guard around rendering the suggestion, so the
information is not linkified if it's untraversable.

Preimp
======
Spoke with Curtis Hovey.

Implementation
==============
Submission, a stub class used to wrap translation messages, has a guard added
to check if the message index is 0.

The TAL checks for this guard, rendering the information as a link if it is
traversable, and as text if not.

Tests
=====
bin/test -vvct test_submission_traversable_guard

QA
==
Ensure that the bad data example in the bug is not rendering as a link.

LoC
===
I have approximately 400 LoC of credit.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/templates/translations-macros.pt
  lib/lp/translations/browser/tests/test_translationmessage_view.py
  lib/lp/translations/browser/translationmessage.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/translations-message-links-404/+merge/131052
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/translations/browser/tests/test_translationmessage_view.py'
--- lib/lp/translations/browser/tests/test_translationmessage_view.py	2012-01-01 02:58:52 +0000
+++ lib/lp/translations/browser/tests/test_translationmessage_view.py	2012-10-23 17:26:21 +0000
@@ -19,6 +19,7 @@
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
     anonymous_logged_in,
+    monkey_patch,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -32,6 +33,7 @@
     CurrentTranslationMessagePageView,
     CurrentTranslationMessageView,
     revert_unselected_translations,
+    convert_translationmessage_to_submission,
     )
 from lp.translations.enums import TranslationPermission
 from lp.translations.interfaces.side import ITranslationSideTraitsSet
@@ -477,3 +479,33 @@
             {1: u''},
             revert_unselected_translations(
                 new_translations, current_message, []))
+
+class TestBadSubmission(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def getSubmission(self, good=True):
+        original_translations = {0: self.getUniqueString()}
+        pofile = self.factory.makePOFile()
+        current = self.factory.makeCurrentTranslationMessage(pofile=pofile)
+        message = self.factory.makeSuggestion(pofile=pofile)
+        if good:
+            message.setPOFile(pofile)
+        submission = convert_translationmessage_to_submission(
+            message=message,
+            current_message=current,
+            plural_form=0,
+            pofile=pofile,
+            legal_warning_needed=False)
+        return submission
+
+    def test_submission_traversable_guard(self):
+        # If a submission doesn't have a sequence greater than 1, it's not
+        # traversable.
+        bad_sub = self.getSubmission(good=False)
+        self.assertEqual(0, bad_sub.translationmessage.sequence)
+        self.assertFalse(bad_sub.is_traversable)
+
+        good_sub = self.getSubmission()
+        self.assertNotEqual(0, good_sub.translationmessage.sequence)
+        self.assertTrue(good_sub.is_traversable)

=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py	2012-08-09 03:41:10 +0000
+++ lib/lp/translations/browser/translationmessage.py	2012-10-23 17:26:21 +0000
@@ -11,6 +11,7 @@
 __all__ = [
     'BaseTranslationView',
     'contains_translations',
+    'convert_translationmessage_to_submission',
     'CurrentTranslationMessageAppMenus',
     'CurrentTranslationMessageFacets',
     'CurrentTranslationMessageIndexView',
@@ -1669,6 +1670,7 @@
     """
 
     submission = Submission()
+    submission.is_traversable = (message.sequence != 0)
     submission.translationmessage = message
     for attribute in ['id', 'language', 'potmsgset', 'date_created']:
         setattr(submission, attribute, getattr(message, attribute))

=== modified file 'lib/lp/translations/templates/translations-macros.pt'
--- lib/lp/translations/templates/translations-macros.pt	2012-07-07 14:00:30 +0000
+++ lib/lp/translations/templates/translations-macros.pt	2012-10-23 17:26:21 +0000
@@ -84,10 +84,13 @@
       <td style="overflow: auto;"
           tal:condition="not:submission/is_local_to_pofile">
         <tal:source content="string:${context/title}" />
-        <a tal:content="submission/pofile/potemplate/displayname"
+        <a tal:condition="submission/is_traversable"
+           tal:content="submission/pofile/potemplate/displayname"
            tal:attributes="href string:${submission/translationmessage/fmt:url}/+translate">
-          Spanish translation for evolution
-        </a> by
+           Spanish translation for evolution </a>
+        <tal:fallback
+          condition="not:submission/is_traversable"
+          content="submission/pofile/potemplate/displayname" /> by
         <a tal:content="submission/person/displayname"
            tal:attributes="href submission/person/fmt:url">Foo Bar</a>
         <span