Merge lp:~danilo/launchpad/bug-401618 into lp:launchpad

[Данило Шеган]

[r=abentley][bug=401618] Finally remove all use of POTMsgSet.sequence in our code, to fix an odd bug or two, and allow for column removal from the database.

= Bug 401618 =

This is a bug about remaining use of POTMsgSet.sequence that stopped us from killing this column in the database: nature of POTMsgSet has changed in the meantime so it can participate in multiple different translation templates, and sequence in each of those templates is stored on TranslationTemplateItem.sequence instead of on POTMsgSet directly.

Biggest issue was the fact that we constructed URLs for TranslationMessages using potmsgset.sequence. To aid with that, we instead introduce TranslationMessage.sequence which uses self.browser_pofile when it's defined, because that's modified according to the current view.

As for all the other uses, I replace potmsgset.sequence with potmsgset.getSequence(proper_template).

The tests previously used to pass because they didn't heavily depend on this change (eg. the fact that potmsgset.setSequence also set potmsgset.sequence was enough for them to pass), so I am not introducing any new tests because plenty would start failing if I did this wrong.

== Tests ==

"bin/test -vvm lp.translations" still passes

== Demo and Q/A ==

Browse around +translate and +filter pages on translations.l.n, especially clicking on the "zoom" icon for each of the messages. Also look for translation suggestions saying "used in" and "suggested in" and follow those links to ensure they point to right pages.

= Launchpad lint =

NOTE: I'll fix lint after the review, to simplify the review.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/doc/vpotexport.txt
  lib/lp/translations/browser/configure.zcml
  lib/lp/translations/interfaces/translationmessage.py
  lib/lp/translations/interfaces/potmsgset.py
  lib/lp/translations/tests/test_pofile.py
  lib/lp/translations/model/pofile.py
  lib/lp/translations/doc/pofile.txt
  lib/lp/translations/browser/tests/translationmessage-views.txt
  lib/lp/translations/model/potmsgset.py
  lib/lp/translations/model/translationmessage.py
  lib/lp/translations/templates/pofile-filter.pt

./lib/lp/translations/doc/vpotexport.txt
       1: narrative uses a moin header.
      18: narrative uses a moin header.
./lib/lp/translations/interfaces/potmsgset.py
     196: Line exceeds 78 characters.
     198: Line exceeds 78 characters.
     343: E251 no spaces around keyword / parameter equals
./lib/lp/translations/model/potmsgset.py
      19: 'IntCol' imported but unused
./lib/lp/translations/templates/pofile-filter.pt
      32: Line has trailing whitespace.

[Aaron Bentley (abentley)]

Looks good to me.

review: Approve

=== modified file 'lib/lp/translations/browser/configure.zcml'
--- lib/lp/translations/browser/configure.zcml	2011-08-25 19:48:19 +0000
+++ lib/lp/translations/browser/configure.zcml	2011-10-06 09:01:25 +0000
@@ -528,7 +528,7 @@
             layer="lp.translations.publisher.TranslationsLayer"/>
         <browser:url
             for="lp.translations.interfaces.translationmessage.ITranslationMessage"
-            path_expression="string:${potmsgset/sequence}"
+            path_expression="string:${sequence}"
             attribute_to_parent="browser_pofile"
             rootsite="translations"/>
         <browser:menus
 
=== modified file 'lib/lp/translations/browser/tests/translationmessage-views.txt'
--- lib/lp/translations/browser/tests/translationmessage-views.txt	2011-01-05 19:32:32 +0000
+++ lib/lp/translations/browser/tests/translationmessage-views.txt	2011-10-06 09:01:25 +0000
@@ -99,8 +99,8 @@
 We are at the beginning because this subview is being used for the first
 item.
 
-    >>> subview.context.potmsgset.sequence == 1
-    True
+    >>> subview.context.potmsgset.getSequence(pofile.potemplate)
+    1
 
 It does not have a plural message
 
@@ -530,8 +530,6 @@
     >>> pofile = factory.makePOFile('sr')
     >>> potemplate = pofile.potemplate
     >>> potmsgset = factory.makePOTMsgSet(potemplate, sequence=1)
-    >>> potmsgset.sequence
-    1
     >>> potmsgset.getSequence(potemplate)
     1
     >>> translationmessage = factory.makeCurrentTranslationMessage(
 
=== modified file 'lib/lp/translations/doc/pofile.txt'
--- lib/lp/translations/doc/pofile.txt	2011-05-27 19:53:20 +0000
+++ lib/lp/translations/doc/pofile.txt	2011-10-06 09:01:25 +0000
@@ -73,7 +73,7 @@
 
 We need a helper method to better display test results.
 
-    >>> def print_potmsgsets(potmsgsets, pofile=None):
+    >>> def print_potmsgsets(potmsgsets, pofile):
     ...     for potmsgset in potmsgsets:
     ...         singular = plural = None
     ...         translation = ""
@@ -86,13 +86,16 @@
     ...             if len(plural) > 20:
     ...                plural = plural[:17] + "..."
     ...         if pofile is not None:
-    ...             translation = potmsgset.getCurrentTranslation(
+    ...             message = potmsgset.getCurrentTranslation(
     ...                 pofile.potemplate, pofile.language,
-    ...                 pofile.potemplate.translation_side).translations[0]
+    ...                 pofile.potemplate.translation_side)
+    ...             if message is not None:
+    ...                translation = message.translations[0]
     ...             if len(translation) > 20:
     ...                 translation = translation[:17] + "..."
     ...         print "%2d. %-20s   %-20s   %-20s" % (
-    ...             potmsgset.sequence, singular, plural, translation)
+    ...             potmsgset.getSequence(pofile.potemplate),
+    ...             singular, plural, translation)
 
 
 getFullLanguageCode
@@ -131,7 +134,7 @@
     >>> found_potmsgsets.count()
     4
 
-    >>> print_potmsgsets(found_potmsgsets)
+    >>> print_potmsgsets(found_potmsgsets, dummy_pofile)
      7. contact's header:      None
     14. The location and ...   None
     15. %d contact             %d contacts
@@ -144,7 +147,7 @@
     >>> found_potmsgsets.count()
     4
 
-    >>> print_potmsgsets(found_potmsgsets)
+    >>> print_potmsgsets(found_potmsgsets, dummy_pofile)
      7. contact's header:      None
     14. The location and ...   None
     15. %d contact             %d contacts
@@ -157,7 +160,7 @@
     >>> found_potmsgsets.count()
     2
 
-    >>> print_potmsgsets(found_potmsgsets)
+    >>> print_potmsgsets(found_potmsgsets, dummy_pofile)
     15. %d contact             %d contacts
     16. Opening %d contac...   Opening %d contac...
 
 
=== modified file 'lib/lp/translations/doc/vpotexport.txt'
--- lib/lp/translations/doc/vpotexport.txt	2009-07-29 18:53:50 +0000
+++ lib/lp/translations/doc/vpotexport.txt	2011-10-06 09:01:25 +0000
@@ -1,4 +1,5 @@
-= Template export sets  =
+Template export sets
+====================
 
 POTemplate.getTranslationRows serialises a template's rows for export.
 
@@ -15,7 +16,8 @@
     True
 
 
-= Template export set entry =
+Template export set entry
+=========================
 
 VPOTExport represent a row for an entry in the POTemplate. Each row has
 the following information, among other things:
@@ -61,7 +63,7 @@
 
     >>> first.sequence
     0
-    >>> potmsgset.sequence
+    >>> potmsgset.getSequence(first.potemplate)
     0
 
 The POT file header is a template of common information that will get filled
 
=== modified file 'lib/lp/translations/interfaces/potmsgset.py'
--- lib/lp/translations/interfaces/potmsgset.py	2011-03-07 07:17:59 +0000
+++ lib/lp/translations/interfaces/potmsgset.py	2011-10-06 09:01:25 +0000
@@ -95,8 +95,6 @@
         readonly=True,
         schema=IPOMsgID)
 
-    sequence = Attribute("The ordering of this set within its file.")
-
     commenttext = Attribute("The manual comments this set has.")
 
     filereferences = Attribute("The files where this set appears.")
@@ -195,9 +193,9 @@
         :param language: language we want translations for.
         """
 
-    def getExternallySuggestedOrUsedTranslationMessages(suggested_languages=(),
-        used_languages=()):
-        """Find externally suggested or used translations for the same message.
+    def getExternallySuggestedOrUsedTranslationMessages(
+        suggested_languages=(), used_languages=()):
+        """Find externally suggested/used translations for the same message.
 
         This returns a mapping: language -> namedtuple (suggested, used)
         containing the results of
@@ -342,7 +340,7 @@
     translation_credits_type = Choice(
         title=u"The type of translation credit of this message.",
         required=True,
-        vocabulary = TranslationCreditsType)
+        vocabulary=TranslationCreditsType)
 
     def makeHTMLID(suffix=None):
         """Unique name for this `POTMsgSet` for use in HTML element ids.
 
=== modified file 'lib/lp/translations/interfaces/translationmessage.py'
--- lib/lp/translations/interfaces/translationmessage.py	2011-02-23 16:22:52 +0000
+++ lib/lp/translations/interfaces/translationmessage.py	2011-10-06 09:01:25 +0000
@@ -113,6 +113,10 @@
         title=_("The translation file from where this translation comes"),
         readonly=False, required=False, schema=IPOFile)
 
+    sequence = Int(
+        title=_("Sequence of a message in the PO file browser_pofile."),
+        readonly=True, required=False)
+
     potemplate = Object(
         title=_("The template this translation is in"),
         readonly=False, required=False, schema=IPOTemplate)
 
=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py	2011-06-22 09:59:13 +0000
+++ lib/lp/translations/model/pofile.py	2011-10-06 09:01:25 +0000
@@ -1118,7 +1118,7 @@
                 error_message = error['error-message']
                 errorsdetails = '%s%d. "%s":\n\n%s\n\n' % (
                     errorsdetails,
-                    potmsgset.sequence,
+                    potmsgset.getSequence(self.potemplate),
                     error_message,
                     pomessage)
 
 
=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py	2011-07-26 11:53:25 +0000
+++ lib/lp/translations/model/potmsgset.py	2011-10-06 09:01:25 +0000
@@ -18,7 +18,6 @@
 
 from sqlobject import (
     ForeignKey,
-    IntCol,
     SQLObjectNotFound,
     StringCol,
     )
@@ -139,7 +138,6 @@
         notNull=True)
     msgid_plural = ForeignKey(foreignKey='POMsgID', dbName='msgid_plural',
         notNull=False, default=DEFAULT)
-    sequence = IntCol(dbName='sequence')
     commenttext = StringCol(dbName='commenttext', notNull=False)
     filereferences = StringCol(dbName='filereferences', notNull=False)
     sourcecomment = StringCol(dbName='sourcecomment', notNull=False)
@@ -1217,7 +1215,6 @@
 
     def setSequence(self, potemplate, sequence):
         """See `IPOTMsgSet`."""
-        self.sequence = sequence
         translation_template_item = TranslationTemplateItem.selectOneBy(
             potmsgset=self, potemplate=potemplate)
         if translation_template_item is not None:
 
=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py	2011-02-24 14:31:46 +0000
+++ lib/lp/translations/model/translationmessage.py	2011-10-06 09:01:25 +0000
@@ -114,6 +114,14 @@
         """See `ITranslationMessage`."""
         self.browser_pofile = pofile
 
+    @property
+    def sequence(self):
+        if self.browser_pofile:
+            pofile = self.browser_pofile
+            return self.potmsgset.getSequence(pofile.potemplate)
+        else:
+            return 0
+
     def markReviewed(self, reviewer, timestamp=None):
         """See `ITranslationMessage`."""
         if timestamp is None:
 
=== modified file 'lib/lp/translations/templates/pofile-filter.pt'
--- lib/lp/translations/templates/pofile-filter.pt	2009-09-14 10:35:23 +0000
+++ lib/lp/translations/templates/pofile-filter.pt	2011-10-06 09:01:25 +0000
@@ -29,7 +29,7 @@
     <tal:translations condition="messages">
       <p tal:condition="view/person">
         <a tal:replace="structure view/person/fmt:link"/> has
-        submitted the following strings to 
+        submitted the following strings to
         <a tal:attributes="href view/context/fmt:url">this translation</a>.
         Contributions are visually coded:
         <span class="usedtranslation">currently used translations</span>,
@@ -46,10 +46,10 @@
         <tal:message repeat="message messages">
           <tr>
             <td class="englishstring" style="text-align:right;">
-              <tal:obsolete condition="not:message/potmsgset/sequence">
+              <tal:obsolete condition="not:message/context/sequence">
                 ~</tal:obsolete>
-              <tal:nonobsolete condition="message/potmsgset/sequence">
-                <a tal:content="string:${message/potmsgset/sequence}."
+              <tal:nonobsolete condition="message/context/sequence">
+                <a tal:content="string:${message/context/sequence}."
                    tal:attributes="href message/context/fmt:url" />
               </tal:nonobsolete>
             </td>
 
=== modified file 'lib/lp/translations/tests/test_pofile.py'
--- lib/lp/translations/tests/test_pofile.py	2011-08-03 11:00:11 +0000
+++ lib/lp/translations/tests/test_pofile.py	2011-10-06 09:01:25 +0000
@@ -1356,32 +1356,6 @@
         self.assertEquals(
             [self.potmsgset1, self.potmsgset2], potmsgsets)
 
-    def test_findPOTMsgSetsContaining_ordering(self):
-        # As per bug 388473 findPOTMsgSetsContaining still used the old
-        # potmsgset.sequence for ordering. Check that this is fixed.
-        # This test will go away when potmsgset.sequence goes away.
-
-        # Give the method something to search for.
-        self.factory.makeCurrentTranslationMessage(
-            pofile=self.devel_pofile,
-            potmsgset=self.potmsgset1,
-            translations=["Shared translation"])
-        self.factory.makeCurrentTranslationMessage(
-            pofile=self.devel_pofile,
-            potmsgset=self.potmsgset2,
-            translations=["Another shared translation"])
-
-        # Mess with potmsgset.sequence.
-        removeSecurityProxy(self.potmsgset1).sequence = 2
-        removeSecurityProxy(self.potmsgset2).sequence = 1
-
-        potmsgsets = list(
-            self.devel_pofile.findPOTMsgSetsContaining("translation"))
-
-        # Order ignores potmsgset.sequence.
-        self.assertEquals(
-            [self.potmsgset1, self.potmsgset2], potmsgsets)
-
 
 class TestPOFileSet(TestCaseWithFactory):
     """Test PO file set methods."""