added test cases for detection of email signature

4 files changed, 131 insertions, 94 deletions

diff --git a/askbot/lamson_handlers.py b/askbot/lamson_handlers.py
index 3b6aefd7..03bdf43a 100644
--- a/askbot/lamson_handlers.py
+++ b/askbot/lamson_handlers.py
@@ -220,14 +220,12 @@ def VALIDATE_EMAIL(
     the email signature
     todo: go a step further and
     """
-    content, stored_files = mail.process_parts(parts)
-    #save the signature and mark email as valid
     reply_code = reply_address_object.address
-    user = reply_address_object.user
-    if reply_code in content:
-        user.email_signature = reply_address_object.extract_user_signature(
-                                                                        content
-                                                                    )
+    try:
+        content, stored_files, signature = mail.process_parts(parts, reply_code)
+        user = reply_address_object.user
+        if signature and signature != user.email_signature:
+            user.email_signature = signature
         user.email_isvalid = True
         user.save()
 
@@ -243,8 +241,7 @@ def VALIDATE_EMAIL(
             body_text = template.render(Context(data)),
             recipient_list = [from_address,]
         )
-
-    else:
+    except ValueError:
         raise ValueError(
             _(
                 'Please reply to the welcome email '
@@ -263,7 +260,19 @@ def PROCESS(
     """handler to process the emailed message
     and make a post to askbot based on the contents of
     the email, including the text body and the file attachments"""
+    #split email into bits
+    reply_code = reply_address_object.address
+    body_text, stored_files, signature = mail.process_parts(parts, reply_code)
+
+    #update signature and validate email address
+    user = reply_address_object.user
+    if signature and signature != user.email_signature:
+        user.email_signature = signature
+    user.email_isvalid = True
+    user.save()#todo: actually, saving is not necessary, if nothing changed
+
     if reply_address_object.was_used:
-        reply_address_object.edit_post(parts)
+        action = reply_address_object.edit_post
     else:
-        reply_address_object.create_reply(parts)
+        action = reply_address_object.create_reply
+    action(body_text, stored_files)
diff --git a/askbot/models/reply_by_email.py b/askbot/models/reply_by_email.py
index 3940709d..058fbbcc 100644
--- a/askbot/models/reply_by_email.py
+++ b/askbot/models/reply_by_email.py
@@ -74,65 +74,32 @@ class ReplyAddress(models.Model):
         """True if was used"""
         return self.used_at != None
 
-    def extract_user_signature(self, text):
-        if self.address in text:
-            #extract the signature
-            tail = list()
-            for line in reversed(text.splitlines()):
-                #scan backwards from the end until the magic line
-                if self.address in line:
-                    break
-                tail.insert(0, line)
-
-            #strip off the leading quoted lines, there could be one or two
-            #also strip empty lines
-            while tail[0].startswith('>') or tail[0].strip() == '':
-                tail.pop(0)
-
-            return '\n'.join(tail)
-        else:
-            return ''
-
-    def edit_post(self, parts):
+    def edit_post(self, body_text, stored_files):
         """edits the created post upon repeated response
         to the same address"""
         assert self.was_used == True
-        content, stored_files = mail.process_parts(parts)
-        content = self.user.strip_email_signature(content)
         self.user.edit_post(
             post = self.response_post,
-            body_text = content,
+            body_text = stored_files,
             revision_comment = _('edited by email'),
             by_email = True
         )
         self.response_post.thread.invalidate_cached_data()
 
-    def create_reply(self, parts):
+    def create_reply(self, body_text, stored_files):
         """creates a reply to the post which was emailed
         to the user
         """
         result = None
-        #todo: delete stored files if this function fails
-        content, stored_files = mail.process_parts(parts)
-
-        if self.address in content:
-            new_signature = self.extract_user_signature(content)
-            if new_signature != self.user.email_signature:
-                self.user.email_signature = new_signature
-                self.user.email_isvalid = True
-                self.user.save()
-
-        content = self.user.strip_email_signature(content)
-
         if self.post.post_type == 'answer':
             result = self.user.post_comment(
                                         self.post,
-                                        content,
+                                        body_text,
                                         by_email = True
                                     )
         elif self.post.post_type == 'question':
             if self.reply_action == 'auto_answer_or_comment':
-                wordcount = len(content)/6#todo: this is a simplistic hack
+                wordcount = len(body_text)/6#todo: this is a simplistic hack
                 if wordcount > askbot_settings.MIN_WORDS_FOR_ANSWER_BY_EMAIL:
                     reply_action = 'post_answer'
                 else:
@@ -143,13 +110,13 @@ class ReplyAddress(models.Model):
             if reply_action == 'post_answer':
                 result = self.user.post_answer(
                                             self.post,
-                                            content,
+                                            body_text,
                                             by_email = True
                                         )
             elif reply_action == 'post_comment':
                 result = self.user.post_comment(
                                             self.post,
-                                            content,
+                                            body_text,
                                             by_email = True
                                         )
             else:
@@ -160,7 +127,7 @@ class ReplyAddress(models.Model):
         elif self.post.post_type == 'comment':
             result = self.user.post_comment(
                                     self.post.parent,
-                                    content,
+                                    body_text,
                                     by_email = True
                                 )
         result.thread.invalidate_cached_data()
diff --git a/askbot/tests/reply_by_email_tests.py b/askbot/tests/reply_by_email_tests.py
index e46d6b3d..b5132dcf 100644
--- a/askbot/tests/reply_by_email_tests.py
+++ b/askbot/tests/reply_by_email_tests.py
@@ -1,6 +1,6 @@
 from django.utils.translation import ugettext as _
 from askbot.models import ReplyAddress
-from askbot.lamson_handlers import PROCESS, get_parts
+from askbot.lamson_handlers import PROCESS, VALIDATE_EMAIL, get_parts
 from askbot import const
 
 
@@ -8,13 +8,7 @@ from askbot.tests.utils import AskbotTestCase
 from askbot.models import Post, PostRevision
 
 TEST_CONTENT = 'Test content'
-TEST_EMAIL_PARTS = (
-    ('body', TEST_CONTENT),
-)
 TEST_LONG_CONTENT = 'Test content' * 10
-TEST_LONG_EMAIL_PARTS = (
-    ('body', TEST_LONG_CONTENT),
-)
 
 class MockPart(object):
     def __init__(self, body):
@@ -23,10 +17,23 @@ class MockPart(object):
 
 class MockMessage(dict):
 
-    def __init__(self, body, from_email):
-        self._body = body
-        self._part = MockPart(body)
+    def __init__(
+        self, content, from_email, signature = '', response_code = False
+    ):
         self.From= from_email
+        self['Subject'] = 'test subject'
+
+        if response_code != False:
+            #in this case we modify the content
+            re_separator = const.REPLY_SEPARATOR_TEMPLATE % {
+                                    'user_action': 'john did something',
+                                    'instruction': 'reply above this line'
+                                }
+            content += '\n\n\nToday someone wrote:\n' + re_separator + \
+                '\nblah blah\n' + response_code + '\n' + signature
+
+        self._body = content
+        self._part = MockPart(content)
 
     def body(self):
         return self._body
@@ -35,7 +42,7 @@ class MockMessage(dict):
         """todo: add real file attachment"""
         return [self._part]
 
-class EmailProcessingTests(AskbotTestCase):
+class ReplyAddressModelTests(AskbotTestCase):
 
     def setUp(self):
         self.u1 = self.create_user(username='user1')
@@ -79,30 +86,6 @@ class EmailProcessingTests(AskbotTestCase):
         self.assertEquals(self.answer.comments.count(), 2)
         self.assertEquals(self.answer.comments.all().order_by('-pk')[0].text.strip(), "This is a test reply")
 
-
-
-class ReplyAddressModelTests(AskbotTestCase):
-
-    def setUp(self):
-        self.u1 = self.create_user(username='user1')
-        self.u1.set_status('a')
-        self.u1.moderate_user_reputation(self.u1, reputation_change = 100, comment=  "no comment")
-        self.u2 = self.create_user(username='user2')
-        self.u1.moderate_user_reputation(self.u2, reputation_change = 100, comment=  "no comment")
-        self.u3 = self.create_user(username='user3')
-        self.u1.moderate_user_reputation(self.u3, reputation_change = 100, comment=  "no comment")
-
-        self.question = self.post_question(
-            user = self.u1,
-            follow = True,
-        )
-        self.answer = self.post_answer(
-            user = self.u2,
-            question = self.question
-        )
-
-        self.comment = self.post_comment(user = self.u2, parent_post = self.answer)
-    
     def test_address_creation(self):
         self.assertEquals(ReplyAddress.objects.all().count(), 0)
         result = ReplyAddress.objects.create_new(
@@ -118,7 +101,7 @@ class ReplyAddressModelTests(AskbotTestCase):
                                         post = self.answer,
                                         user = self.u1
                                     )
-        post = result.create_reply(TEST_EMAIL_PARTS)
+        post = result.create_reply(TEST_CONTENT, [])
         self.assertEquals(post.post_type, "comment")
         self.assertEquals(post.text, TEST_CONTENT)
         self.assertEquals(self.answer.comments.count(), 2)
@@ -128,7 +111,7 @@ class ReplyAddressModelTests(AskbotTestCase):
                                         post = self.comment,
                                         user = self.u1
                                     )
-        post = result.create_reply(TEST_EMAIL_PARTS)
+        post = result.create_reply(TEST_CONTENT, [])
         self.assertEquals(post.post_type, "comment")
         self.assertEquals(post.text, TEST_CONTENT)
         self.assertEquals(self.answer.comments.count(), 2)
@@ -139,7 +122,7 @@ class ReplyAddressModelTests(AskbotTestCase):
                                         post = self.question,
                                         user = self.u3
                                     )
-        post = result.create_reply(TEST_EMAIL_PARTS)
+        post = result.create_reply(TEST_CONTENT, [])
         self.assertEquals(post.post_type, "comment")
         self.assertEquals(post.text, TEST_CONTENT)
 
@@ -148,6 +131,58 @@ class ReplyAddressModelTests(AskbotTestCase):
                                         post = self.question,
                                         user = self.u3
                                     )
-        post = result.create_reply(TEST_LONG_EMAIL_PARTS)
+        post = result.create_reply(TEST_LONG_CONTENT, [])
         self.assertEquals(post.post_type, "answer")
         self.assertEquals(post.text, TEST_LONG_CONTENT)
+
+class EmailSignatureDetectionTests(AskbotTestCase):
+    
+    def setUp(self):
+        self.u1 = self.create_user('user1', status = 'a')
+        self.u2 = self.create_user('user2', status = 'a')
+
+    def test_detect_signature_in_response(self):
+        question = self.post_question(user = self.u1)
+
+        #create a response address record
+        reply_token = ReplyAddress.objects.create_new(
+                                        post = question,
+                                        user = self.u2,
+                                        reply_action = 'post_answer'
+                                    )
+
+        self.u2.email_signature = ''
+        self.u2.save()
+
+        msg = MockMessage(
+                'some text',
+                self.u2.email,
+                signature = 'Yours Truly',
+                response_code = reply_token.address
+            )
+        PROCESS(msg, address = reply_token.address)
+
+        signature = self.reload_object(self.u2).email_signature 
+        self.assertEqual(signature, 'Yours Truly')
+
+    def test_detect_signature_in_welcome_response(self):
+        reply_token = ReplyAddress.objects.create_new(
+                                            user = self.u2,
+                                            reply_action = 'validate_email'
+                                        )
+        self.u2.email_signature = ''
+        self.u2.save()
+
+        msg = MockMessage(
+                'some text',
+                self.u2.email,
+                signature = 'Yours Truly',
+                response_code = reply_token.address
+            )
+        VALIDATE_EMAIL(
+            msg,
+            address = reply_token.address
+        )
+
+        signature = self.reload_object(self.u2).email_signature 
+        self.assertEqual(signature, 'Yours Truly')
diff --git a/askbot/utils/mail.py b/askbot/utils/mail.py
index 4f653e6b..17eaf52c 100644
--- a/askbot/utils/mail.py
+++ b/askbot/utils/mail.py
@@ -250,7 +250,29 @@ def process_attachment(attachment):
         markdown_link = '!' + markdown_link
     return markdown_link, file_storage
 
-def process_parts(parts):
+def extract_user_signature(text, reply_code):
+    """extracts email signature as text trailing
+    the reply code"""
+    if reply_code in text:
+        #extract the signature
+        tail = list()
+        for line in reversed(text.splitlines()):
+            #scan backwards from the end until the magic line
+            if reply_code in line:
+                break
+            tail.insert(0, line)
+
+        #strip off the leading quoted lines, there could be one or two
+        #also strip empty lines
+        while tail[0].startswith('>') or tail[0].strip() == '':
+            tail.pop(0)
+
+        return '\n'.join(tail)
+    else:
+        return ''
+
+
+def process_parts(parts, reply_code = None):
     """Process parts will upload the attachments and parse out the
     body, if body is multipart. Secondly - links to attachments
     will be added to the body of the question.
@@ -274,10 +296,14 @@ def process_parts(parts):
 
     #if the response separator is present - 
     #split the body with it, and discard the "so and so wrote:" part
+    if reply_code:
+        signature = extract_user_signature(body_markdown, reply_code)
+    else:
+        signature = None
     body_markdown = extract_reply(body_markdown)
 
     body_markdown += attachments_markdown
-    return body_markdown.strip(), stored_files
+    return body_markdown.strip(), stored_files, signature
 
 
 def process_emailed_question(from_address, subject, parts, tags = None):
@@ -288,7 +314,7 @@ def process_emailed_question(from_address, subject, parts, tags = None):
 
     try:
         #todo: delete uploaded files when posting by email fails!!!
-        body, stored_files = process_parts(parts)
+        body, stored_files, unused = process_parts(parts)
         data = {
             'sender': from_address,
             'subject': subject,