Merge lp:~lifeless/launchpad/registry into lp:launchpad

[Robert Collins]

Fix mailing list moderation UFD's.

Fix unexpected form data in mailing list moderation.

[Robert Collins (lifeless)]

Note, this is tested by the tests for moderation; specific tests could be added but I don't think they'll add any coverage or value. (migrating the higher level tests down would add value, but only if we /move/ not /add/.

[Curtis Hovey (sinzui)]

This looks good to land.

review: Approve (code)

[Brad Crittenden (bac)]

Robert,

Per our shiny new multi-line import rule, could you fix line 9 please?

:)

--Brad

[Robert Collins (lifeless)]

The thing is Brad, that line 9 is actually ok in bzr, which is what I was referencing ;)

I will change it anyhow, but I think we can easily spend a lot of cycles on trivial stuff.

=== 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 18:36:33 +0000
@@ -25,7 +25,10 @@
 
 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 +841,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 18:36:33 +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 18:36:33 +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`."""