launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15627
[Merge] lp:~stevenk/launchpad/reject-reason-plus-queue into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/reject-reason-plus-queue into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #271413 in Launchpad itself: "Queue UI should allow the user to provide a a rejection reason"
https://bugs.launchpad.net/launchpad/+bug/271413
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/reject-reason-plus-queue/+merge/165960
Make use of the new comment argument to IPackageUpload.rejectFromQueue() by showing a textbox for a reason and require it to be specified when rejecting packages.
--
https://code.launchpad.net/~stevenk/launchpad/reject-reason-plus-queue/+merge/165960
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/reject-reason-plus-queue into lp:launchpad.
=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py 2013-04-17 08:13:18 +0000
+++ lib/lp/soyuz/browser/queue.py 2013-05-28 05:45:36 +0000
@@ -309,6 +309,7 @@
# Retrieve the form data.
accept = self.request.form.get('Accept', '')
reject = self.request.form.get('Reject', '')
+ rejection_comment = self.request.form.get('rejection_comment', '')
component_override = self.request.form.get('component_override', '')
section_override = self.request.form.get('section_override', '')
priority_override = self.request.form.get('priority_override', '')
@@ -318,6 +319,11 @@
if (not accept and not reject) or not queue_ids:
return
+ # If we're asked to reject with no comment, bail.
+ if reject and not rejection_comment:
+ self.error = 'Require a comment when rejecting queue items.'
+ return
+
# Determine if there is a source override requested.
new_component = None
new_section = None
@@ -421,7 +427,8 @@
'priority'] = new_priority.title.lower()
try:
- getattr(self, 'queue_action_' + action)(queue_item)
+ getattr(self, 'queue_action_' + action)(
+ queue_item, rejection_comment)
except (QueueAdminUnauthorizedError,
QueueInconsistentStateError) as info:
failure.append('FAILED: %s (%s)' %
@@ -447,13 +454,13 @@
url = str(self.request.URL) + "?queue_state=%s" % self.state.value
self.request.response.redirect(url)
- def queue_action_accept(self, queue_item):
+ def queue_action_accept(self, queue_item, comment):
"""Accept the queue item passed."""
queue_item.acceptFromQueue(user=self.user)
- def queue_action_reject(self, queue_item):
+ def queue_action_reject(self, queue_item, comment):
"""Reject the queue item passed."""
- queue_item.rejectFromQueue(user=self.user)
+ queue_item.rejectFromQueue(user=self.user, comment=comment)
def sortedSections(self):
"""Possible sections for the context distroseries.
=== modified file 'lib/lp/soyuz/browser/tests/test_queue.py'
--- lib/lp/soyuz/browser/tests/test_queue.py 2013-04-17 11:07:33 +0000
+++ lib/lp/soyuz/browser/tests/test_queue.py 2013-05-28 05:45:36 +0000
@@ -132,6 +132,11 @@
view.performQueueAction()
return view
+ def assertStatus(self, package_upload_id, status):
+ self.assertEqual(
+ status,
+ getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+
def test_main_admin_can_accept_main_upload(self):
# A person with queue admin access for main
# can accept uploads to the main archive.
@@ -145,10 +150,7 @@
request = LaunchpadTestRequest(form=self.form)
request.method = 'POST'
self.setupQueueView(request)
-
- self.assertEquals(
- 'DONE',
- getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ self.assertStatus(package_upload_id, 'DONE')
def test_main_admin_cannot_accept_partner_upload(self):
# A person with queue admin access for main cannot necessarily
@@ -169,9 +171,7 @@
"FAILED: partner-upload (You have no rights to accept "
"component(s) 'partner')"),
view.request.response.notifications[0].message)
- self.assertEquals(
- 'NEW',
- getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ self.assertStatus(package_upload_id, 'NEW')
def test_admin_can_accept_partner_upload(self):
# An admin can always accept packages, even for the
@@ -183,10 +183,7 @@
request = LaunchpadTestRequest(form=self.form)
request.method = 'POST'
self.setupQueueView(request)
-
- self.assertEquals(
- 'DONE',
- getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ self.assertStatus(package_upload_id, 'DONE')
def test_partner_admin_can_accept_partner_upload(self):
# A person with queue admin access for partner
@@ -201,10 +198,7 @@
request = LaunchpadTestRequest(form=self.form)
request.method = 'POST'
self.setupQueueView(request)
-
- self.assertEquals(
- 'DONE',
- getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ self.assertStatus(package_upload_id, 'DONE')
def test_partner_admin_cannot_accept_main_upload(self):
# A person with queue admin access for partner cannot necessarily
@@ -225,9 +219,7 @@
"FAILED: main-upload (You have no rights to accept "
"component(s) 'main')"),
view.request.response.notifications[0].message)
- self.assertEquals(
- 'NEW',
- getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ self.assertStatus(package_upload_id, 'NEW')
def test_proposed_admin_can_accept_proposed_upload(self):
# A person with queue admin access for proposed can accept uploads
@@ -243,7 +235,6 @@
self.proposed_queue_admin,
pocket=PackagePublishingPocket.PROPOSED,
distroseries=distroseries))
- package_upload_set = getUtility(IPackageUploadSet)
for spr in (self.proposed_spr, self.proposed_series_spr):
package_upload_id = spr.package_upload.id
@@ -251,9 +242,7 @@
request = LaunchpadTestRequest(form=self.form)
request.method = 'POST'
self.setupQueueView(request, series=spr.upload_distroseries)
-
- self.assertEqual(
- 'DONE', package_upload_set.get(package_upload_id).status.name)
+ self.assertStatus(package_upload_id, 'DONE')
def test_proposed_admin_cannot_accept_release_upload(self):
# A person with queue admin access for proposed cannot necessarly
@@ -275,9 +264,7 @@
"FAILED: main-upload (You have no rights to accept "
"component(s) 'main')"),
view.request.response.notifications[0].message)
- self.assertEqual(
- 'NEW',
- getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ self.assertStatus(package_upload_id, 'NEW')
def test_proposed_series_admin_can_accept_that_series_upload(self):
# A person with queue admin access for proposed for one series can
@@ -294,10 +281,7 @@
request = LaunchpadTestRequest(form=self.form)
request.method = 'POST'
self.setupQueueView(request, series=self.second_series)
-
- self.assertEqual(
- 'DONE',
- getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ self.assertStatus(package_upload_id, 'DONE')
def test_proposed_series_admin_cannot_accept_other_series_upload(self):
# A person with queue admin access for proposed for one series
@@ -317,9 +301,37 @@
self.assertEqual(
"You do not have permission to act on queue items.", view.error)
+ self.assertStatus(package_upload_id, 'NEW')
+
+
+class TestRejectQueueUploads(TestAcceptQueueUploads):
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_cannot_reject_without_comment(self):
+ login_person(self.proposed_queue_admin)
+ package_upload_id = self.proposed_spr.package_upload.id
+ form = {
+ 'Reject': 'Reject',
+ 'QUEUE_ID': [package_upload_id]}
+ request = LaunchpadTestRequest(form=form)
+ request.method = 'POST'
+ view = self.setupQueueView(request)
self.assertEqual(
- 'NEW',
- getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ 'Require a comment when rejecting queue items.', view.error)
+ self.assertStatus(package_upload_id, 'NEW')
+
+ def test_reject_with_comment(self):
+ login_person(self.proposed_queue_admin)
+ package_upload_id = self.proposed_spr.package_upload.id
+ form = {
+ 'Reject': 'Reject',
+ 'rejection_comment': 'Because I can.',
+ 'QUEUE_ID': [package_upload_id]}
+ request = LaunchpadTestRequest(form=form)
+ request.method = 'POST'
+ self.setupQueueView(request)
+ self.assertStatus(package_upload_id, 'REJECTED')
class TestQueueItemsView(TestCaseWithFactory):
=== modified file 'lib/lp/soyuz/templates/distroseries-queue.pt'
--- lib/lp/soyuz/templates/distroseries-queue.pt 2013-04-18 08:49:33 +0000
+++ lib/lp/soyuz/templates/distroseries-queue.pt 2013-05-28 05:45:36 +0000
@@ -183,15 +183,25 @@
</select>
</td>
<td colspan="2" style="vertical-align: bottom">
+ <input value="Accept" name="Accept" type="submit"/>
+ </td>
+ </tr>
+ <tr tal:condition="view/availableActions">
+ <td colspan="4" style="text-align: right;
+ vertical-align: bottom;
+ padding-bottom: 5px">
+ <label>Rejection comment:</label>
+ </td>
+ <td colspan="3" style="vertical-align: bottom">
+ <input size="80" type="text" name="rejection_comment" />
+ </td>
+ <td style="vertical-align: bottom">
<input
tal:condition="view/state/enumvalue:REJECTED"
disabled="true" value="Reject" name="Reject" type="submit"/>
<input
tal:condition="not:view/state/enumvalue:REJECTED"
value="Reject" name="Reject" type="submit"/>
- <br/>
- <br/>
- <input value="Accept" name="Accept" type="submit"/>
</td>
</tr>
</tbody>
