Merge lp:~danci-emanuel/mailman/messages_support into lp:~danci-emanuel/mailman/dlist_runner

[Emanuel Danci]

[Emanuel Danci (danci-emanuel)]

Just some small comments:

1. This is only the part that will be used by the handlers when creating the new messags. (the equivalent of the Message class from DlistUtils)

2. I did not push the code for the other parts of the DlistUtils (the ones related to the Thread and Override classes) because I still have to finish some parts of them and I also took you advice, pushing smaller pieces of code, so that they will be easier to review.

3. I also changed the attribute from mailinglist.py, the one that you told me to, and added the missing line to max_size.py.

4. I tried to add Terri as I reviewer to, but I cannot seem to find her on launchpad.

[Robin J (robin-jeffries)]

> Just some small comments:
>
> 1. This is only the part that will be used by the handlers when creating the
> new messags. (the equivalent of the Message class from DlistUtils)

OK, I don't think I understand why existing mailman doesn't include the msgdata, but you need to. Can you explain it to me? (It's probably because I don't know the mm3 code, but it will help us both if you can explain it.) What is in the metadata?

>
> 2. I did not push the code for the other parts of the DlistUtils (the ones
> related to the Thread and Override classes) because I still have to finish
> some parts of them and I also took you advice, pushing smaller pieces of code,
> so that they will be easier to review.

Good.

>
> 3. I also changed the attribute from mailinglist.py, the one that you told me
> to, and added the missing line to max_size.py.

You mean the help text associated with that attribute?

>
> 4. I tried to add Terri as I reviewer to, but I cannot seem to find her on
> launchpad.

I can't figure out how to add another reviewer from this view, but Terri is terriko on launchpad.

[Emanuel Danci (danci-emanuel)]

> > Just some small comments:
> >
> > 1. This is only the part that will be used by the handlers when creating the
> > new messags. (the equivalent of the Message class from DlistUtils)
>
> OK, I don't think I understand why existing mailman doesn't include the
> msgdata, but you need to. Can you explain it to me? (It's probably because I
> don't know the mm3 code, but it will help us both if you can explain it.)
> What is in the metadata?
>

Right not, mm3 only stores the following information for a certain message: the message_id_hash, its path and the message_id, which are all found in the msg param. I added the msgdata because it contains the 'thread_id' field, which will be added by the one of the two handlers (either FirstDlist or SecondDlist, as Sneha Priscilla called them), when the message gets there.

> >
> > 2. I did not push the code for the other parts of the DlistUtils (the ones
> > related to the Thread and Override classes) because I still have to finish
> > some parts of them and I also took you advice, pushing smaller pieces of
> code,
> > so that they will be easier to review.
>
> Good.
>
> >
> > 3. I also changed the attribute from mailinglist.py, the one that you told
> me
> > to, and added the missing line to max_size.py.
>
> You mean the help text associated with that attribute?

Yes, the text.

> >
> > 4. I tried to add Terri as I reviewer to, but I cannot seem to find her on
> > launchpad.
>
> I can't figure out how to add another reviewer from this view, but Terri is
> terriko on launchpad.

Thank you for Terri`s id! I added her to the reviewers list, too!

[Robin J (robin-jeffries)]

> > > Just some small comments:
> > >
> > > 1. This is only the part that will be used by the handlers when creating
> the
> > > new messags. (the equivalent of the Message class from DlistUtils)
> >
> > OK, I don't think I understand why existing mailman doesn't include the
> > msgdata, but you need to. Can you explain it to me? (It's probably because
> I
> > don't know the mm3 code, but it will help us both if you can explain it.)
> > What is in the metadata?
> >
>
> Right not, mm3 only stores the following information for a certain message:
> the message_id_hash, its path and the message_id, which are all found in the
> msg param. I added the msgdata because it contains the 'thread_id' field,
> which will be added by the one of the two handlers (either FirstDlist or
> SecondDlist, as Sneha Priscilla called them), when the message gets there.
>

So, msg is an object, so why not just make the thread_id an attribute of the object? (I don't want us to get to far away from what mailman is doing, and that seems to be what mailman is doing with the other info about the message)

I'm refering to
  moderator.py
  messages.py

[Emanuel Danci (danci-emanuel)]

> > > > Just some small comments:
> > > >
> > > > 1. This is only the part that will be used by the handlers when creating
> > the
> > > > new messags. (the equivalent of the Message class from DlistUtils)
> > >
> > > OK, I don't think I understand why existing mailman doesn't include the
> > > msgdata, but you need to. Can you explain it to me? (It's probably
> because
> > I
> > > don't know the mm3 code, but it will help us both if you can explain it.)
> > > What is in the metadata?
> > >
> >
> > Right not, mm3 only stores the following information for a certain message:
> > the message_id_hash, its path and the message_id, which are all found in the
> > msg param. I added the msgdata because it contains the 'thread_id' field,
> > which will be added by the one of the two handlers (either FirstDlist or
> > SecondDlist, as Sneha Priscilla called them), when the message gets there.
> >
>
> So, msg is an object, so why not just make the thread_id an attribute of the
> object? (I don't want us to get to far away from what mailman is doing, and
> that seems to be what mailman is doing with the other info about the message)
>
> I'm refering to
> moderator.py
> messages.py

Yes, theoretically that can be done. So than I should go with changing the initial dlist model and add the 'thread_id' to msg instead of adding it to the msgdata? This means that I will only have to keep in mind to search the thread_id in the msg object instead of the msgdata one, when doing the rest of the actions from DlistUtils.

[Robin J (robin-jeffries)]

> > So, msg is an object, so why not just make the thread_id an attribute of the
> > object? (I don't want us to get to far away from what mailman is doing, and
> > that seems to be what mailman is doing with the other info about the
> message)
> >
> > I'm refering to
> > moderator.py
> > messages.py
>
> Yes, theoretically that can be done. So than I should go with changing the
> initial dlist model and add the 'thread_id' to msg instead of adding it to the
> msgdata? This means that I will only have to keep in mind to search the
> thread_id in the msg object instead of the msgdata one, when doing the rest of
> the actions from DlistUtils.

Yes, this makes it easier for you, and doesn't confuse a future reader of the code about why thread_id is different from other data related to the message. (The old dlists were a private addon, so they wanted to keep as much separate from existing mm objects as possible, but this work is going into core mailman 3, so it's more important that it be parallel to other parts of that code)

Unmerged revisions

7165. By Emanuel Danci on 2012-09-17

Added the missing part from the messages.

=== modified file 'src/mailman/app/moderator.py'
--- src/mailman/app/moderator.py	2012-03-15 22:48:41 +0000
+++ src/mailman/app/moderator.py	2012-09-17 21:12:24 +0000
@@ -90,7 +90,7 @@
         msg['Message-ID'] = message_id = unicode(make_msgid())
     assert isinstance(message_id, unicode), (
         'Message-ID is not a unicode: %s' % message_id)
-    getUtility(IMessageStore).add(msg)
+    getUtility(IMessageStore).add(msg, msgdata)
     # Prepare the message metadata with some extra information needed only by
     # the moderation interface.
     msgdata['_mod_message_id'] = message_id
 
=== modified file 'src/mailman/database/schema/postgres.sql'
--- src/mailman/database/schema/postgres.sql	2012-09-11 18:17:03 +0000
+++ src/mailman/database/schema/postgres.sql	2012-09-17 21:12:24 +0000
@@ -291,6 +291,7 @@
     message_id_hash BYTEA,
     path BYTEA,
     message_id TEXT,
+    thread_id_dlist INTEGER,
     PRIMARY KEY (id)
     );
 
 
=== modified file 'src/mailman/database/schema/sqlite.sql'
--- src/mailman/database/schema/sqlite.sql	2012-09-11 18:17:03 +0000
+++ src/mailman/database/schema/sqlite.sql	2012-09-17 21:12:24 +0000
@@ -239,6 +239,7 @@
     message_id_hash TEXT,
     path TEXT,
     message_id TEXT,
+    thread_id_dlist INTEGER,
     PRIMARY KEY (id)
     );
 
 
=== modified file 'src/mailman/interfaces/mailinglist.py'
--- src/mailman/interfaces/mailinglist.py	2012-09-11 18:17:03 +0000
+++ src/mailman/interfaces/mailinglist.py	2012-09-17 21:12:24 +0000
@@ -133,7 +133,7 @@
         available mailing lists.""")
 
     dlists_enabled = Attribute(
-	    """Is the Dynamic sublists feature enabled for this list?""")
+	    """Allow people to (un)subscribe to individual conversations?""")
 	
     # Contact addresses
 
 
=== modified file 'src/mailman/interfaces/messages.py'
--- src/mailman/interfaces/messages.py	2012-01-01 19:14:46 +0000
+++ src/mailman/interfaces/messages.py	2012-09-17 21:12:24 +0000
@@ -62,12 +62,13 @@
     http://archive.example.com/RXTJ357KFOTJP3NFJA6KMO65X7VQOHJI
     """
 
-    def add(message):
+    def add(message, msgdata):
         """Add the message to the store.
 
         :param message: An email.message.Message instance containing at least
             a unique Message-ID header.  The message will be given an
             X-Message-ID-Hash header, overriding any existing such header.
+        :param msgdata: The message metadata.    
         :returns: The calculated X-Message-ID-Hash header.
         :raises ValueError: if the message is missing a Message-ID header.
             The storage service is also allowed to raise this exception if it
@@ -109,3 +110,6 @@
     message_id_hash = Attribute("""The unique SHA1 hash of the message.""")
 
     path = Attribute("""The filesystem path to the message object.""")
+    
+    thread_id_dlist = Attribute("""The id of the thread that the message
+        belongs to.""")
 
=== modified file 'src/mailman/model/message.py'
--- src/mailman/model/message.py	2012-04-26 02:08:22 +0000
+++ src/mailman/model/message.py	2012-09-17 21:12:24 +0000
@@ -41,12 +41,15 @@
     message_id = Unicode()
     message_id_hash = RawStr()
     path = RawStr()
+    thread_id_dlist = Int()
     # This is a Messge-ID field representation, not a database row id.
 
     @dbconnection
-    def __init__(self, store, message_id, message_id_hash, path):
+    def __init__(self, store, message_id, message_id_hash, path, 
+            thread_id_dlist):
         super(Message, self).__init__()
         self.message_id = message_id
         self.message_id_hash = message_id_hash
         self.path = path
+        self.thread_id_dlist = thread_id_dlist
         store.add(self)
 
=== modified file 'src/mailman/model/messagestore.py'
--- src/mailman/model/messagestore.py	2012-04-26 02:08:22 +0000
+++ src/mailman/model/messagestore.py	2012-09-17 21:12:24 +0000
@@ -51,9 +51,12 @@
     """See `IMessageStore`."""
 
     @dbconnection
-    def add(self, store, message):
+    def add(self, store, message, msgdata):
+        return
         # Ensure that the message has the requisite headers.
         message_ids = message.get_all('message-id', [])
+        for i in range(len(message_ids)):
+            print( message_ids[i] )
         if len(message_ids) <> 1:
             raise ValueError('Exactly one Message-ID header required')
         # Calculate and insert the X-Message-ID-Hash.
@@ -76,13 +79,19 @@
             parts.append(split.pop(0) + split.pop(0))
         parts.append(hash32)
         relpath = os.path.join(*parts)
+        # extract the thread_id
+        try:
+            thread_id = msgdata['thread_id']
+        except:
+            thread_id = -1
         # Store the message in the database.  This relies on the database
         # providing a unique serial number, but to get this information, we
         # have to use a straight insert instead of relying on Elixir to create
         # the object.
         row = Message(message_id=message_id,
                       message_id_hash=hash32,
-                      path=relpath)
+                      path=relpath,
+                      thread_id_dlist=thread_id)
         # Now calculate the full file system path.
         path = os.path.join(config.MESSAGES_DIR, relpath)
         # Write the file to the path, but catch the appropriate exception in
 
=== modified file 'src/mailman/rules/max_size.py'
--- src/mailman/rules/max_size.py	2012-09-11 17:20:51 +0000
+++ src/mailman/rules/max_size.py	2012-09-17 21:12:24 +0000
@@ -3,6 +3,7 @@
 # This file is part of GNU Mailman.
 #
 # GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
 # Software Foundation, either version 3 of the License, or (at your option)
 # any later version.
 #