launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/registry into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #615237 /participants API timing out
  https://bugs.launchpad.net/bugs/615237
  #627412 Person:+mailinglist-moderate timeout : error while trying to moderate messages
  https://bugs.launchpad.net/bugs/627412
  #637654 upper and lower batch navigators create duplicate and invalid ids
  https://bugs.launchpad.net/bugs/637654
  #640700 UnexpectedFormData: Invalid moderation action for held message 
  https://bugs.launchpad.net/bugs/640700


Fix unexpected form data in mailing list moderation.
-- 
https://code.launchpad.net/~lifeless/launchpad/registry/+merge/35774
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/registry into lp:launchpad/devel.

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2010-09-14 00:21:04 +0000
+++ lib/lp/registry/browser/team.py	2010-09-17 04:12:46 +0000
@@ -25,7 +25,7 @@
 
 from datetime import datetime
 import math
-from urllib import quote
+from urllib import quote, unquote
 
 import pytz
 from zope.app.form.browser import TextAreaWidget
@@ -838,9 +838,19 @@
         # won't be in data.  Instead, get it out of the request.
         reviewable = self.hold_count
         disposed_count = 0
-        for message in self.held_messages.currentBatch():
-            action_name = self.request.form_ng.getOne(
-                'field.' + quote(message.message_id))
+        actions = {}
+        form = self.request.form_ng
+        for field_name in form:
+            if (field_name.startswith('field.') and
+                field_name.endswith('')):
+                # A moderated message.
+                quoted_id = field_name[len('field.'):]
+                message_id = unquote(quoted_id)
+                actions[message_id] = form.getOne(field_name)
+        messages = self.mailing_list.getReviewableMessages(
+            message_id_filter=actions)
+        for message in messages:
+            action_name = actions[message.message_id]
             # This essentially acts like a switch statement or if/elifs.  It
             # looks the action up in a map of allowed actions, watching out
             # for bogus input.

=== modified file 'lib/lp/registry/interfaces/mailinglist.py'
--- lib/lp/registry/interfaces/mailinglist.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/interfaces/mailinglist.py	2010-09-17 04:12:46 +0000
@@ -431,9 +431,11 @@
         :return: The IMessageApproval representing the held message.
         """
 
-    def getReviewableMessages():
+    def getReviewableMessages(message_id_filter=None):
         """Return the set of all held messages for this list requiring review.
 
+        :param message_id_filter: If supplied only messages with message ids in
+            the filter are returned.
         :return: A sequence of `IMessageApproval`s for this mailing list,
             where the status is `PostedMessageStatus.NEW`.  The returned set
             is ordered first by the date the message was posted, then by

=== modified file 'lib/lp/registry/model/mailinglist.py'
--- lib/lp/registry/model/mailinglist.py	2010-09-03 03:12:39 +0000
+++ lib/lp/registry/model/mailinglist.py	2010-09-17 04:12:46 +0000
@@ -21,6 +21,7 @@
     make_header,
     )
 from itertools import repeat
+import operator
 from socket import getfqdn
 from string import Template
 
@@ -63,8 +64,10 @@
     sqlvalues,
     )
 from canonical.launchpad import _
+from canonical.launchpad.components.decoratedresultset import DecoratedResultSet
 from canonical.launchpad.database.account import Account
 from canonical.launchpad.database.emailaddress import EmailAddress
+from canonical.launchpad.database.message import Message
 from canonical.launchpad.interfaces.account import AccountStatus
 from canonical.launchpad.interfaces.emailaddress import (
     EmailAddressStatus,
@@ -557,15 +560,20 @@
         notify(ObjectCreatedEvent(held_message))
         return held_message
 
-    def getReviewableMessages(self):
+    def getReviewableMessages(self, message_id_filter=None):
         """See `IMailingList`."""
-        return MessageApproval.select("""
-            MessageApproval.mailing_list = %s AND
-            MessageApproval.status = %s AND
-            MessageApproval.message = Message.id
-            """ % sqlvalues(self, PostedMessageStatus.NEW),
-            clauseTables=['Message'],
-            orderBy=['posted_date', 'Message.rfc822msgid'])
+        store = Store.of(self)
+        clauses = [
+            MessageApproval.mailing_listID==self.id,
+            MessageApproval.status==PostedMessageStatus.NEW,
+            MessageApproval.messageID==Message.id,
+            ]
+        if message_id_filter is not None:
+            clauses.append(Message.rfc822msgid.is_in(message_id_filter))
+        results = store.find((MessageApproval, Message),
+            *clauses)
+        results.order_by(MessageApproval.posted_date, Message.rfc822msgid)
+        return DecoratedResultSet(results, operator.itemgetter(0))
 
     def purge(self):
         """See `IMailingList`."""