launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/bmp-remove-old-comment-threading into lp:launchpad.

Commit message:
Remove BranchMergeProposalView.comments and the underlying message threading support.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bmp-remove-old-comment-threading/+merge/344197

It's superseded by BranchMergeProposalView.conversation.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bmp-remove-old-comment-threading into lp:launchpad.

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2018-03-16 21:20:00 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2018-04-24 23:07:17 +0000
@@ -102,7 +102,6 @@
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
 from lp.services.librarian.interfaces.client import LibrarianServerError
-from lp.services.messages.interfaces.message import IMessageSet
 from lp.services.propertycache import (
     cachedproperty,
     get_property_cache,
@@ -706,27 +705,6 @@
                 comment).previewdiff_id = relations.get(comment.id)
 
     @property
-    def comments(self):
-        """Return comments associated with this proposal, plus styling info.
-
-        Comments are in threaded order, and the style indicates indenting
-        for use with threads.
-        """
-        message_to_comment = {}
-        messages = []
-        for comment in self.context.all_comments:
-            message_to_comment[comment.message] = comment
-            messages.append(comment.message)
-        message_set = getUtility(IMessageSet)
-        threads = message_set.threadMessages(messages)
-        result = []
-        for depth, message in message_set.flattenThreads(threads):
-            comment = message_to_comment[message]
-            style = 'margin-left: %dem;' % (2 * depth)
-            result.append(dict(style=style, comment=comment))
-        return result
-
-    @property
     def label(self):
         return "Merge %s into %s" % (
             self.context.merge_source.identity,

=== modified file 'lib/lp/services/messages/interfaces/message.py'
--- lib/lp/services/messages/interfaces/message.py	2015-07-09 20:06:17 +0000
+++ lib/lp/services/messages/interfaces/message.py	2018-04-24 23:07:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -165,27 +165,6 @@
             * InvalidEmailMessage
         """
 
-    def threadMessages(messages):
-        """Return a threaded version of supplied message list.
-
-        Return value is a recursive list structure.
-        Each parent entry in the top-level list is a tuple of
-        (parent, children), where children is a list of parents.  (Parents
-        may be childless.)
-
-        Example:
-        [(parent, [(child1, [(grandchild1, [])]), (child2, [])])]
-        """
-
-    def flattenThreads(threaded_messages):
-        """Convert threaded messages into a flat, indented form.
-
-        Take a thread (in the form produced by threadMessages) and
-        iterate through a series of (depth, message) tuples.  The ordering
-        will match that implied by the input structure, with all replies
-        to a message appearing after that message.
-        """
-
 
 class IIndexedMessage(Interface):
     """An `IMessage` decorated with its index and context."""

=== modified file 'lib/lp/services/messages/model/message.py'
--- lib/lp/services/messages/model/message.py	2015-07-08 16:05:11 +0000
+++ lib/lp/services/messages/model/message.py	2018-04-24 23:07:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -483,42 +483,6 @@
         Store.of(message).flush()
         return message
 
-    @staticmethod
-    def _parentToChild(messages):
-        """Return a mapping from parent to child and list of root messages."""
-        result = {}
-        roots = []
-        for message in messages:
-            if message.parent is None:
-                roots.append(message)
-            else:
-                result.setdefault(message.parent, []).append(message)
-            result.setdefault(message, [])
-        return result, roots
-
-    @classmethod
-    def threadMessages(klass, messages):
-        """See `IMessageSet`."""
-        result, roots = klass._parentToChild(messages)
-
-        def get_children(node):
-            children = []
-            for child in result[node]:
-                children.append((child, get_children(child)))
-            return children
-        threads = []
-        for root in roots:
-            threads.append((root, get_children(root)))
-        return threads
-
-    @classmethod
-    def flattenThreads(klass, threaded_messages, _depth=0):
-        """See `IMessageSet`."""
-        for message, children in threaded_messages:
-            yield (_depth, message)
-            for depth, message in klass.flattenThreads(children, _depth + 1):
-                yield depth, message
-
 
 @implementer(IMessageChunk)
 class MessageChunk(SQLBase):

=== modified file 'lib/lp/services/messages/tests/test_message.py'
--- lib/lp/services/messages/tests/test_message.py	2014-01-30 15:04:06 +0000
+++ lib/lp/services/messages/tests/test_message.py	2018-04-24 23:07:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -42,36 +42,6 @@
         message4 = self.factory.makeMessage(parent=message2)
         return (message1, message2, message3, message4)
 
-    def test_parentToChild(self):
-        """Test MessageSet._parentToChild."""
-        messages = self.createTestMessages()
-        message1, message2, message3, message4 = messages
-        expected = {
-            message1: [message2, message3],
-            message2: [message4],
-            message3: [], message4: []}
-        result, roots = MessageSet._parentToChild(messages)
-        self.assertEqual(expected, result)
-        self.assertEqual([message1], roots)
-
-    def test_threadMessages(self):
-        """Test MessageSet.threadMessages."""
-        messages = self.createTestMessages()
-        message1, message2, message3, message4 = messages
-        threads = MessageSet.threadMessages(messages)
-        self.assertEqual(
-            [(message1, [(message2, [(message4, [])]), (message3, [])])],
-            threads)
-
-    def test_flattenThreads(self):
-        """Test MessageSet.flattenThreads."""
-        messages = self.createTestMessages()
-        message1, message2, message3, message4 = messages
-        threads = MessageSet.threadMessages(messages)
-        flattened = list(MessageSet.flattenThreads(threads))
-        expected = [(0, message1), (1, message2), (2, message4), (1, message3)]
-        self.assertEqual(expected, flattened)
-
     def _makeMessageWithAttachment(self, filename='review.diff'):
         sender = self.factory.makePerson()
         msg = MIMEMultipart()