Launchpad itself

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

  1. bug-516739-needs-information
  2. Merge into devel
Proposed by Henning Eggers
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~henninge/launchpad/bug-516739-needs-information
Merge into: lp:launchpad
Diff against target: 201 lines (+40/-25)
6 files modified
lib/canonical/launchpad/icing/style-3-0.css (+2/-0)
lib/lp/translations/doc/translationimportqueue.txt (+1/-0)
lib/lp/translations/interfaces/translationimportqueue.py (+6/-0)
lib/lp/translations/model/translationimportqueue.py (+4/-0)
lib/lp/translations/stories/importqueue/xx-translation-import-queue-filtering.txt (+1/-0)
lib/lp/translations/tests/test_translationimportqueue.py (+26/-25)
To merge this branch: bzr merge lp:~henninge/launchpad/bug-516739-needs-information
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) code Approve
Review via email: mp+18693@code.launchpad.net

Commit message

Added Need Information status for translation import queue.

To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :

-----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-----

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

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
Revision history for this message
Jeroen T. Vermeulen (jtv) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
655.translationimportstatusNEEDS_REVIEW a {color: #f60;}655.translationimportstatusNEEDS_REVIEW a {color: #f60;}
656.translationimportstatusBLOCKED,656.translationimportstatusBLOCKED,
657.translationimportstatusBLOCKED a {color: blue;}657.translationimportstatusBLOCKED a {color: blue;}
658.translationimportstatusNEEDS_INFORMATION,
659.translationimportstatusNEEDS_INFORMATION a {color: maroon;}
658660
659/* ==== Translations hand-made forms ==== */661/* ==== Translations hand-made forms ==== */
660662
661663
=== 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 @@
1461 debian/woody Woody1461 debian/woody Woody
1462 BLOCKED:1462 BLOCKED:
1463 ubuntu/grumpy Grumpy1463 ubuntu/grumpy Grumpy
1464 NEEDS_INFORMATION:
14641465
1465You can also select by status.1466You can also select by status.
14661467
14671468
=== 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 @@
104 The entry has been blocked to be imported by a Rosetta Expert.104 The entry has been blocked to be imported by a Rosetta Expert.
105 """)105 """)
106106
107 NEEDS_INFORMATION = DBItem(7, """
108 Needs Information
109
110 The reviewer needs more information before this entry can be approved.
111 """)
112
107113
108class SpecialTranslationImportTargetFilter(DBEnumeratedType):114class SpecialTranslationImportTargetFilter(DBEnumeratedType):
109 """Special "meta-targets" to filter the queue view by."""115 """Special "meta-targets" to filter the queue view by."""
110116
=== 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 @@
320 # that's only possible if we know where to import it320 # that's only possible if we know where to import it
321 # (import_into not None).321 # (import_into not None).
322 return self.canAdmin(roles) and self.import_into is not None322 return self.canAdmin(roles) and self.import_into is not None
323 if new_status == RosettaImportStatus.NEEDS_INFORMATION:
324 # Only administrators are able to set the NEEDS_INFORMATION
325 # status.
326 return self.canAdmin(roles)
323 if new_status == RosettaImportStatus.IMPORTED:327 if new_status == RosettaImportStatus.IMPORTED:
324 # Only rosetta experts are able to set the IMPORTED status, and328 # Only rosetta experts are able to set the IMPORTED status, and
325 # that's only possible if we know where to import it329 # that's only possible if we know where to import it
326330
=== 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 @@
84 Failed84 Failed
85 Needs Review85 Needs Review
86 Blocked86 Blocked
87 Needs Information
8788
88 >>> print_batch_heading(browser)89 >>> print_batch_heading(browser)
89 1 ... 5 of 102 results90 1 ... 5 of 102 results
9091
=== 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 @@
45 def _assertCanSetStatus(self, user, entry, expected_list):45 def _assertCanSetStatus(self, user, entry, expected_list):
46 # Helper to check for all statuses.46 # Helper to check for all statuses.
47 # Could iterate RosettaImportStatus.items but listing them here47 # Could iterate RosettaImportStatus.items but listing them here
48 # explicitely is better to read. They are sorted alphabetically.48 # explicitly is better to read. They are sorted alphabetically.
49 possible_statuses = [49 possible_statuses = [
50 RosettaImportStatus.APPROVED,50 RosettaImportStatus.APPROVED,
51 RosettaImportStatus.BLOCKED,51 RosettaImportStatus.BLOCKED,
52 RosettaImportStatus.DELETED,52 RosettaImportStatus.DELETED,
53 RosettaImportStatus.FAILED,53 RosettaImportStatus.FAILED,
54 RosettaImportStatus.IMPORTED,54 RosettaImportStatus.IMPORTED,
55 RosettaImportStatus.NEEDS_INFORMATION,
55 RosettaImportStatus.NEEDS_REVIEW,56 RosettaImportStatus.NEEDS_REVIEW,
56 ]57 ]
57 self._switch_dbuser()58 self._switch_dbuser()
@@ -64,14 +65,14 @@
64 # A non-privileged users cannot set any status.65 # A non-privileged users cannot set any status.
65 some_user = self.factory.makePerson()66 some_user = self.factory.makePerson()
66 self._assertCanSetStatus(some_user, self.entry,67 self._assertCanSetStatus(some_user, self.entry,
67 # A B D F I NR68 # A B D F I NI NR
68 [False, False, False, False, False, False])69 [False, False, False, False, False, False, False])
6970
70 def test_canSetStatus_rosetta_expert(self):71 def test_canSetStatus_rosetta_expert(self):
71 # Rosetta experts are all-powerful, didn't you know that?72 # Rosetta experts are all-powerful, didn't you know that?
72 self._assertCanSetStatus(self.rosetta_experts, self.entry,73 self._assertCanSetStatus(self.rosetta_experts, self.entry,
73 # A B D F I NR74 # A B D F I NI NR
74 [True, True, True, True, True, True])75 [True, True, True, True, True, True, True])
7576
76 def test_canSetStatus_rosetta_expert_no_target(self):77 def test_canSetStatus_rosetta_expert_no_target(self):
77 # If the entry has no import target set, even Rosetta experts78 # If the entry has no import target set, even Rosetta experts
@@ -79,56 +80,56 @@
79 self.entry.potemplate = None80 self.entry.potemplate = None
80 self.entry.pofile = None81 self.entry.pofile = None
81 self._assertCanSetStatus(self.rosetta_experts, self.entry,82 self._assertCanSetStatus(self.rosetta_experts, self.entry,
82 # A B D F I NR83 # A B D F I NI NR
83 [False, True, True, True, False, True])84 [False, True, True, True, False, True, True])
8485
85 def test_canSetStatus_uploader(self):86 def test_canSetStatus_uploader(self):
86 # The uploader can set some statuses.87 # The uploader can set some statuses.
87 self._assertCanSetStatus(self.uploaderperson, self.entry,88 self._assertCanSetStatus(self.uploaderperson, self.entry,
88 # A B D F I NR89 # A B D F I NI NR
89 [False, False, True, False, False, True])90 [False, False, True, False, False, False, True])
9091
91 def test_canSetStatus_product_owner(self):92 def test_canSetStatus_product_owner(self):
92 # The owner (maintainer) of the product gets to set Blocked as well.93 # The owner (maintainer) of the product gets to set Blocked as well.
93 owner = self.productseries.product.owner94 owner = self.productseries.product.owner
94 self._assertCanSetStatus(owner, self.entry,95 self._assertCanSetStatus(owner, self.entry,
95 # A B D F I NR96 # A B D F I NI NR
96 [False, True, True, False, False, True])97 [False, True, True, False, False, False, True])
9798
98 def test_canSetStatus_owner_and_uploader(self):99 def test_canSetStatus_owner_and_uploader(self):
99 # Corner case: Nothing changes if the maintainer is also the uploader.100 # Corner case: Nothing changes if the maintainer is also the uploader.
100 self.productseries.product.owner = self.uploaderperson101 self.productseries.product.owner = self.uploaderperson
101 self._assertCanSetStatus(self.uploaderperson, self.entry,102 self._assertCanSetStatus(self.uploaderperson, self.entry,
102 # A B D F I NR103 # A B D F I NI NR
103 [False, True, True, False, False, True])104 [False, True, True, False, False, False, True])
104105
105 def test_canSetStatus_driver(self):106 def test_canSetStatus_driver(self):
106 # The driver gets the same permissions as the maintainer.107 # The driver gets the same permissions as the maintainer.
107 driver = self.productseries.driver108 driver = self.productseries.driver
108 self._assertCanSetStatus(driver, self.entry,109 self._assertCanSetStatus(driver, self.entry,
109 # A B D F I NR110 # A B D F I NI NR
110 [False, True, True, False, False, True])111 [False, True, True, False, False, False, True])
111112
112 def test_canSetStatus_driver_and_uploader(self):113 def test_canSetStatus_driver_and_uploader(self):
113 # Corner case: Nothing changes if the driver is also the uploader.114 # Corner case: Nothing changes if the driver is also the uploader.
114 self.productseries.driver = self.uploaderperson115 self.productseries.driver = self.uploaderperson
115 self._assertCanSetStatus(self.uploaderperson, self.entry,116 self._assertCanSetStatus(self.uploaderperson, self.entry,
116 # A B D F I NR117 # A B D F I NI NR
117 [False, True, True, False, False, True])118 [False, True, True, False, False, False, True])
118119
119 def test_canSetStatus_product_driver(self):120 def test_canSetStatus_product_driver(self):
120 # The driver of the product, too.121 # The driver of the product, too.
121 driver = self.productseries.product.driver122 driver = self.productseries.product.driver
122 self._assertCanSetStatus(driver, self.entry,123 self._assertCanSetStatus(driver, self.entry,
123 # A B D F I NR124 # A B D F I NI NR
124 [False, True, True, False, False, True])125 [False, True, True, False, False, False, True])
125126
126 def test_canSetStatus_product_driver_and_uploader(self):127 def test_canSetStatus_product_driver_and_uploader(self):
127 # Corner case: Nothing changes if the driver is also the uploader.128 # Corner case: Nothing changes if the driver is also the uploader.
128 self.productseries.product.driver = self.uploaderperson129 self.productseries.product.driver = self.uploaderperson
129 self._assertCanSetStatus(self.uploaderperson, self.entry,130 self._assertCanSetStatus(self.uploaderperson, self.entry,
130 # A B D F I NR131 # A B D F I NI NR
131 [False, True, True, False, False, True])132 [False, True, True, False, False, False, True])
132133
133 def _setUpUbuntu(self):134 def _setUpUbuntu(self):
134 self.ubuntu = getUtility(ILaunchpadCelebrities).ubuntu135 self.ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
@@ -146,16 +147,16 @@
146 sourcepackagename=self.factory.makeSourcePackageName(),147 sourcepackagename=self.factory.makeSourcePackageName(),
147 potemplate=self.potemplate)148 potemplate=self.potemplate)
148 self._assertCanSetStatus(self.ubuntu_group_owner, ubuntu_entry,149 self._assertCanSetStatus(self.ubuntu_group_owner, ubuntu_entry,
149 # A B D F I NR150 # A B D F I NI NR
150 [True, True, True, False, False, True])151 [True, True, True, False, False, True, True])
151152
152 def test_canSetStatus_ubuntu_translation_group_not_ubuntu(self):153 def test_canSetStatus_ubuntu_translation_group_not_ubuntu(self):
153 # Outside of Ubuntu, owners of the Ubuntu translation Groups have no154 # Outside of Ubuntu, owners of the Ubuntu translation Groups have no
154 # powers.155 # powers.
155 self._setUpUbuntu()156 self._setUpUbuntu()
156 self._assertCanSetStatus(self.ubuntu_group_owner, self.entry,157 self._assertCanSetStatus(self.ubuntu_group_owner, self.entry,
157 # A B D F I NR158 # A B D F I NI NR
158 [False, False, False, False, False, False])159 [False, False, False, False, False, False, False])
159160
160161
161class TestCanSetStatusPOTemplate(TestCanSetStatusBase):162class TestCanSetStatusPOTemplate(TestCanSetStatusBase):