Merge lp:~henninge/launchpad/bug-516739-needs-information into lp:launchpad

[Henning Eggers]

Added Need Information status for translation import queue.

[Henning Eggers (henninge)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

= Summary =

Fixes bug 516739 by introducing a new status for translation import
queue entries called "Needs Information" to request a reply from the
uploader.

== Proposed fix ==

Add the new status to the RosettaImportStatus enum and add permissions
to TranslationImportQueueEntry.canSetStatus.

== Pre-implementation notes ==

Agreed with jtv that "Needs Information" is a better name than
"Incomplete" because it prompts the uploader for action. Also agreed,
that permissions should be the same as for "Approved".

== Implementation details ==

Actually, the permissions are not identical to "Approved" because "Needs
Information" can also be set if no import target has been selected.

== Tests ==

bin/test -vvct canSetSTatus

== Demo and Q/A ==

Go to the import queue as an admin and you'll see the new status in the
status picker.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
lib/lp/translations/model/translationimportqueue.py
lib/canonical/launchpad/icing/style-3-0.css
lib/lp/translations/tests/test_translationimportqueue.py
lib/lp/translations/interfaces/translationimportqueue.py

== Pylint notices ==

lib/lp/translations/interfaces/translationimportqueue.py
10: [F0401] Unable to import 'lazr.enum' (No module named enum)
20: [F0401] Unable to import 'lazr.restful.interface' (No module named
restful)
21: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
22: [F0401] Unable to import 'lazr.restful.declarations' (No module
named restful)

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAktr7KIACgkQBT3oW1L17igkkgCfYxNNZGLZSIft6Z3ys8iYLolN
S/UAn0a/mPAJn5oHPnoVHDHHFJFWQu93
=OnOh
-----END PGP SIGNATURE-----

[Jeroen T. Vermeulen (jtv)]

Reviewed by IRC. Small fixes:
 * Better docstring for the new state.
 * The explicit ordering of states used in the test was no longer alphabetical; being made alphabetical again.
 * Pre-existing typo in comment. :-)

review: Approve

[Jeroen T. Vermeulen (jtv)]

review: Approve (code)

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2010-01-21 17:55:18 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2010-02-05 15:31:17 +0000
@@ -655,6 +655,8 @@
 .translationimportstatusNEEDS_REVIEW a {color: #f60;}
 .translationimportstatusBLOCKED,
 .translationimportstatusBLOCKED a {color: blue;}
+.translationimportstatusNEEDS_INFORMATION,
+.translationimportstatusNEEDS_INFORMATION a {color: maroon;}
 
 /* ==== Translations hand-made forms ==== */
 

=== modified file 'lib/lp/translations/doc/translationimportqueue.txt'
--- lib/lp/translations/doc/translationimportqueue.txt	2010-01-26 15:25:53 +0000
+++ lib/lp/translations/doc/translationimportqueue.txt	2010-02-05 15:31:17 +0000
@@ -1461,6 +1461,7 @@
     debian/woody        Woody
     BLOCKED:
     ubuntu/grumpy       Grumpy
+    NEEDS_INFORMATION:
 
 You can also select by status.
 

=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
--- lib/lp/translations/interfaces/translationimportqueue.py	2010-02-03 10:17:08 +0000
+++ lib/lp/translations/interfaces/translationimportqueue.py	2010-02-05 15:31:17 +0000
@@ -104,6 +104,12 @@
         The entry has been blocked to be imported by a Rosetta Expert.
         """)
 
+    NEEDS_INFORMATION = DBItem(7, """
+        Needs Information
+
+        The reviewer needs more information before this entry can be approved.
+        """)
+
 
 class SpecialTranslationImportTargetFilter(DBEnumeratedType):
     """Special "meta-targets" to filter the queue view by."""

=== modified file 'lib/lp/translations/model/translationimportqueue.py'
--- lib/lp/translations/model/translationimportqueue.py	2010-02-03 12:08:47 +0000
+++ lib/lp/translations/model/translationimportqueue.py	2010-02-05 15:31:17 +0000
@@ -320,6 +320,10 @@
             # that's only possible if we know where to import it
             # (import_into not None).
             return self.canAdmin(roles) and self.import_into is not None
+        if new_status == RosettaImportStatus.NEEDS_INFORMATION:
+            # Only administrators are able to set the NEEDS_INFORMATION
+            # status.
+            return self.canAdmin(roles)
         if new_status == RosettaImportStatus.IMPORTED:
             # Only rosetta experts are able to set the IMPORTED status, and
             # that's only possible if we know where to import it

=== modified file 'lib/lp/translations/stories/importqueue/xx-translation-import-queue-filtering.txt'
--- lib/lp/translations/stories/importqueue/xx-translation-import-queue-filtering.txt	2009-09-01 21:03:51 +0000
+++ lib/lp/translations/stories/importqueue/xx-translation-import-queue-filtering.txt	2010-02-05 15:31:17 +0000
@@ -84,6 +84,7 @@
     Failed
     Needs Review
     Blocked
+    Needs Information
 
     >>> print_batch_heading(browser)
     1 ... 5 of 102 results

=== modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
--- lib/lp/translations/tests/test_translationimportqueue.py	2010-02-03 12:08:47 +0000
+++ lib/lp/translations/tests/test_translationimportqueue.py	2010-02-05 15:31:17 +0000
@@ -45,13 +45,14 @@
     def _assertCanSetStatus(self, user, entry, expected_list):
         # Helper to check for all statuses.
         # Could iterate RosettaImportStatus.items but listing them here
-        # explicitely is better to read. They are sorted alphabetically.
+        # explicitly is better to read. They are sorted alphabetically.
         possible_statuses = [
             RosettaImportStatus.APPROVED,
             RosettaImportStatus.BLOCKED,
             RosettaImportStatus.DELETED,
             RosettaImportStatus.FAILED,
             RosettaImportStatus.IMPORTED,
+            RosettaImportStatus.NEEDS_INFORMATION,
             RosettaImportStatus.NEEDS_REVIEW,
         ]
         self._switch_dbuser()
@@ -64,14 +65,14 @@
         # A non-privileged users cannot set any status.
         some_user = self.factory.makePerson()
         self._assertCanSetStatus(some_user, self.entry,
-            #  A      B      D      F      I     NR
-            [False, False, False, False, False, False])
+            #  A      B      D      F      I     NI     NR
+            [False, False, False, False, False, False, False])
 
     def test_canSetStatus_rosetta_expert(self):
         # Rosetta experts are all-powerful, didn't you know that?
         self._assertCanSetStatus(self.rosetta_experts, self.entry,
-            #  A     B     D     F     I    NR
-            [True, True, True, True, True, True])
+            #  A     B     D     F     I    NI    NR
+            [True, True, True, True, True, True, True])
 
     def test_canSetStatus_rosetta_expert_no_target(self):
         # If the entry has no import target set, even Rosetta experts
@@ -79,56 +80,56 @@
         self.entry.potemplate = None
         self.entry.pofile = None
         self._assertCanSetStatus(self.rosetta_experts, self.entry,
-            #  A      B     D     F     I    NR
-            [False, True, True, True, False, True])
+            #  A      B     D     F     I    NI     NR
+            [False, True, True, True, False, True, True])
 
     def test_canSetStatus_uploader(self):
         # The uploader can set some statuses.
         self._assertCanSetStatus(self.uploaderperson, self.entry,
-            #  A      B     D     F      I     NR
-            [False, False, True, False, False, True])
+            #  A      B     D     F      I     NI     NR
+            [False, False, True, False, False, False, True])
 
     def test_canSetStatus_product_owner(self):
         # The owner (maintainer) of the product gets to set Blocked as well.
         owner = self.productseries.product.owner
         self._assertCanSetStatus(owner, self.entry,
-            #  A      B     D     F      I     NR
-            [False, True, True, False, False, True])
+            #  A      B     D     F      I     NI     NR
+            [False, True, True, False, False, False, True])
 
     def test_canSetStatus_owner_and_uploader(self):
         # Corner case: Nothing changes if the maintainer is also the uploader.
         self.productseries.product.owner = self.uploaderperson
         self._assertCanSetStatus(self.uploaderperson, self.entry,
-            #  A      B     D     F      I     NR
-            [False, True, True, False, False, True])
+            #  A      B     D     F      I     NI     NR
+            [False, True, True, False, False, False, True])
 
     def test_canSetStatus_driver(self):
         # The driver gets the same permissions as the maintainer.
         driver = self.productseries.driver
         self._assertCanSetStatus(driver, self.entry,
-            #  A      B     D     F      I     NR
-            [False, True, True, False, False, True])
+            #  A      B     D     F      I     NI     NR
+            [False, True, True, False, False, False, True])
 
     def test_canSetStatus_driver_and_uploader(self):
         # Corner case: Nothing changes if the driver is also the uploader.
         self.productseries.driver = self.uploaderperson
         self._assertCanSetStatus(self.uploaderperson, self.entry,
-            #  A      B     D     F      I     NR
-            [False, True, True, False, False, True])
+            #  A      B     D     F      I     NI     NR
+            [False, True, True, False, False, False, True])
 
     def test_canSetStatus_product_driver(self):
         # The driver of the product, too.
         driver = self.productseries.product.driver
         self._assertCanSetStatus(driver, self.entry,
-            #  A      B     D     F      I     NR
-            [False, True, True, False, False, True])
+            #  A      B     D     F      I     NI     NR
+            [False, True, True, False, False, False, True])
 
     def test_canSetStatus_product_driver_and_uploader(self):
         # Corner case: Nothing changes if the driver is also the uploader.
         self.productseries.product.driver = self.uploaderperson
         self._assertCanSetStatus(self.uploaderperson, self.entry,
-            #  A      B     D     F      I     NR
-            [False, True, True, False, False, True])
+            #  A      B     D     F      I     NI     NR
+            [False, True, True, False, False, False, True])
 
     def _setUpUbuntu(self):
         self.ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
@@ -146,16 +147,16 @@
             sourcepackagename=self.factory.makeSourcePackageName(),
             potemplate=self.potemplate)
         self._assertCanSetStatus(self.ubuntu_group_owner, ubuntu_entry,
-            #  A     B     D     F      I     NR
-            [True, True, True, False, False, True])
+            #  A     B     D     F      I     NI    NR
+            [True, True, True, False, False, True, True])
 
     def test_canSetStatus_ubuntu_translation_group_not_ubuntu(self):
         # Outside of Ubuntu, owners of the Ubuntu translation Groups have no
         # powers.
         self._setUpUbuntu()
         self._assertCanSetStatus(self.ubuntu_group_owner, self.entry,
-            #  A      B      D      F      I     NR
-            [False, False, False, False, False, False])
+            #  A      B      D      F      I     NI     NR
+            [False, False, False, False, False, False, False])
 
 
 class TestCanSetStatusPOTemplate(TestCanSetStatusBase):