Merge pull request #501 from gwasser/487_new_test

Updated get_email tests

Makefile

@@ -17,12 +17,12 @@ help:
 	@egrep -o "^#: (.+)" [Mm]akefile  | sed 's/#: /* /'
 
 
-#: develop - Install minimal development utilities for Python3
+#: develop - Install minimal development utilities for Python3.
 .PHONY: develop
 develop:
 	$(PIP) install -e .
 
-#: develop - Install minimal development utilities for Python2
+#: develop2 - Install minimal development utilities for Python2.
 .PHONY: develop2
 develop2:
 	pip2 install -e .
@@ -62,7 +62,7 @@ test:
 	$(TOX)
 
 
-#: documentation - Build documentation (Sphinx, README, ...)
+#: documentation - Build documentation (Sphinx, README, ...).
 .PHONY: documentation
 documentation: sphinx readme
 
@@ -93,7 +93,7 @@ demo:
 	demodesk loaddata emailtemplate.json
 	demodesk loaddata demo.json
 
-#: demo - Setup demo project using Python2.
+#: demo2 - Setup demo project using Python2.
 .PHONY: demo2
 demo2:
 	pip2 install -e .
@@ -108,12 +108,12 @@ demo2:
 	demodesk loaddata demo.json
 
 
-#: runserver - Run demo server using Python3
+#: rundemo - Run demo server using Python3.
 .PHONY: rundemo
 rundemo: demo
 	demodesk runserver 8080
 
-#: runserver - Run demo server using Python2
+#: rundemo2 - Run demo server using Python2.
 .PHONY: rundemo2
 rundemo2: demo2
 	demodesk runserver 8080

demo/demodesk/config/settings.py

@@ -39,6 +39,7 @@ INSTALLED_APPS = [
     'django.contrib.staticfiles',
     'django.contrib.sites',
     'django.contrib.humanize',
+    'markdown_deux',
     'bootstrapform',
     'helpdesk'
 ]

demo/demodesk/fixtures/demo.json

@@ -9,5 +9,10 @@
     {"model": "helpdesk.followup", "pk": 3, "fields": {"ticket": 3, "date": "2017-03-20T05:14:36.345Z", "title": "Ticket Opened", "comment": "WHOA!", "public": true, "user": 1, "new_status": null}},
     {"model": "helpdesk.attachment", "pk": 1, "fields": {"followup": 3, "file": "helpdesk/attachments/DH-3/3/someinfo.txt", "filename": "someinfo.txt", "mime_type": "text/plain", "size": 56}},
     {"model": "helpdesk.followup", "pk": 4, "fields": {"ticket": 3, "date": "2017-03-20T05:28:28.458Z", "title": "Comment", "comment": "An image attachment goes here!", "public": true, "user": 1, "new_status": null}},
-    {"model": "helpdesk.attachment", "pk": 2, "fields": {"followup": 4, "file": "helpdesk/attachments/DH-3/4/helpdesk.png", "filename": "helpdesk.png", "mime_type": "image/png", "size": 30229}}
+    {"model": "helpdesk.attachment", "pk": 2, "fields": {"followup": 4, "file": "helpdesk/attachments/DH-3/4/helpdesk.png", "filename": "helpdesk.png", "mime_type": "image/png", "size": 30229}},
+    {"model": "helpdesk.kbcategory", "pk": 1, "fields": {"title": "KB Cat 1", "slug": "kbcat1", "description": "Some category of KB info"}},
+    {"model": "helpdesk.kbcategory", "pk": 2, "fields": {"title": "KB Cat 2", "slug": "kbcat2", "description": "Here is another category. Enjoy!"}},
+    {"model": "helpdesk.kbitem", "pk": 1, "fields": {"category": 1, "title": "Django-Helpdesk", "question": "What is Django-Helpdesk?", "answer": "An open source helpdesk written in python using the awesome django framework.", "votes": 0, "recommendations": 0, "last_updated": "2017-04-02T19:02:17.213Z"}},
+    {"model": "helpdesk.kbitem", "pk": 2, "fields": {"category": 1, "title": "Contributing to django-helpdesk", "question": "How do I contribute?", "answer": "Read the CONTRIBUTING.md file in the top directory of the django-helpdesk source.", "votes": 0, "recommendations": 0, "last_updated": "2017-04-02T19:02:48.374Z"}},
+    {"model": "helpdesk.kbitem", "pk": 3, "fields": {"category": 2, "title": "Something Else", "question": "What else?", "answer": "Not sure.", "votes": 0, "recommendations": 0, "last_updated": "2017-04-02T19:02:59.741Z"}}
 ]

helpdesk/management/commands/get_email.py

@@ -10,6 +10,7 @@ scripts/get_email.py - Designed to be run from cron, this script checks the
                        helpdesk, creating tickets from the new messages (or
                        adding to existing tickets if needed)
 """
+from __future__ import unicode_literals
 
 from datetime import timedelta
 import email
@@ -158,7 +159,7 @@ def process_queue(q, logger):
             msgNum = msg.split(" ")[0]
             logger.info("Processing message %s" % msgNum)
 
-            full_message = "\n".join(server.retr(msgNum)[1])
+            full_message = encoding.force_text("\n".join(server.retr(msgNum)[1]), errors='replace')
             ticket = ticket_from_message(message=full_message, queue=q, logger=logger)
 
             if ticket:
@@ -198,7 +199,8 @@ def process_queue(q, logger):
             for num in msgnums:
                 logger.info("Processing message %s" % num)
                 status, data = server.fetch(num, '(RFC822)')
-                ticket = ticket_from_message(message=encoding.smart_text(data[0][1], errors='replace'), queue=q, logger=logger)
+                full_message = encoding.force_text(data[0][1], errors='replace')
+                ticket = ticket_from_message(message=full_message, queue=q, logger=logger)
                 if ticket:
                     server.store(num, '+FLAGS', '\\Deleted')
                     logger.info("Successfully processed message %s, deleted from IMAP server" % num)
@@ -218,7 +220,8 @@ def process_queue(q, logger):
         for i, m in enumerate(mail, 1):
             logger.info("Processing message %d" % i)
             with open(m, 'r') as f:
-                ticket = ticket_from_message(message=f.read(), queue=q, logger=logger)
+                full_message = encoding.force_text(f.read(), errors='replace')
+                ticket = ticket_from_message(message=full_message, queue=q, logger=logger)
             if ticket:
                 logger.info("Successfully processed message %d, ticket/comment created." % i)
                 try:
@@ -235,9 +238,9 @@ def decodeUnknown(charset, string):
     if six.PY2:
         if not charset:
             try:
-                return string.decode('utf-8', 'ignore')
+                return string.decode('utf-8', 'replace')
             except:
-                return string.decode('iso8859-1', 'ignore')
+                return string.decode('iso8859-1', 'replace')
         return unicode(string, charset)
     elif six.PY3:
         if type(string) is not str:
@@ -251,7 +254,7 @@ def decodeUnknown(charset, string):
 
 
 def decode_mail_headers(string):
-    decoded = email.header.decode_header(string)
+    decoded = email.header.decode_header(string) if six.PY3 else email.header.decode_header(string.encode('utf-8'))
     if six.PY2:
         return u' '.join([unicode(msg, charset or 'utf-8') for msg, charset in decoded])
     elif six.PY3:
@@ -260,7 +263,7 @@ def decode_mail_headers(string):
 
 def ticket_from_message(message, queue, logger):
     # 'message' must be an RFC822 formatted message.
-    message = email.message_from_string(message)
+    message = email.message_from_string(message) if six.PY3 else email.message_from_string(message.encode('utf-8'))
     subject = message.get('subject', _('Created from e-mail'))
     subject = decode_mail_headers(decodeUnknown(message.get_charset(), subject))
     for affix in STRIPPED_SUBJECT_STRINGS:
@@ -305,6 +308,8 @@ def ticket_from_message(message, queue, logger):
                 body = EmailReplyParser.parse_reply(
                     decodeUnknown(part.get_content_charset(), part.get_payload(decode=True))
                 )
+                # workaround to get unicode text out rather than escaped text
+                body = body.encode('ascii').decode('unicode_escape') if six.PY3 else body.encode('utf-8')
                 logger.debug("Discovered plain text MIME part")
             else:
                 files.append(

helpdesk/tests/test_get_email.py

@@ -1,4 +1,8 @@
-from helpdesk.models import Queue, Ticket
+# -*- coding: utf-8 -*-
+
+from __future__ import unicode_literals
+
+from helpdesk.models import Queue, Ticket, FollowUp, Attachment
 from django.test import TestCase
 from django.core.management import call_command
 from django.utils import six
@@ -40,7 +44,7 @@ class GetEmailCommonTests(TestCase):
 
 
 class GetEmailParametricTemplate(object):
-    """TestCase that checks email functionality accross methods and socks configs."""
+    """TestCase that checks email functionality across methods and socks configs."""
 
     def setUp(self):
 
@@ -52,7 +56,8 @@ class GetEmailParametricTemplate(object):
             "allow_email_submission": True,
             "email_box_type": self.method,
             "logging_dir": self.temp_logdir,
-            "logging_type": 'none'}
+            "logging_type": 'none'
+        }
 
         if self.method == 'local':
             kwargs["email_box_local_dir"] = '/var/lib/mail/helpdesk/'
@@ -71,11 +76,16 @@ class GetEmailParametricTemplate(object):
 
         rmtree(self.temp_logdir)
 
-    def test_read_email(self):
+    def test_read_plain_email(self):
-        """Tests reading emails from a queue and creating tickets.
+        """Tests reading plain text emails from a queue and creating tickets.
            For each email source supported, we mock the backend to provide
-           authenticly formatted responses containing our test data."""
+           authentically formatted responses containing our test data."""
-        test_email = "To: update.public@example.com\nFrom: comment@example.com\nSubject: Some Comment\n\nThis is the helpdesk comment via email."
+
+        # example email text from Django docs: https://docs.djangoproject.com/en/1.10/ref/unicode/
+        test_email_from = "Arnbjörg Ráðormsdóttir <arnbjorg@example.com>"
+        test_email_subject = "My visit to Sør-Trøndelag"
+        test_email_body = "Unicode helpdesk comment with an s-hat (ŝ) via email."
+        test_email = "To: helpdesk@example.com\nFrom: " + test_email_from + "\nSubject: " + test_email_subject + "\n\n" + test_email_body
         test_mail_len = len(test_email)
 
         if self.socks:
@@ -130,11 +140,133 @@ class GetEmailParametricTemplate(object):
 
             ticket1 = get_object_or_404(Ticket, pk=1)
             self.assertEqual(ticket1.ticket_for_url, "QQ-%s" % ticket1.id)
-            self.assertEqual(ticket1.description, "This is the helpdesk comment via email.")
+            self.assertEqual(ticket1.title, test_email_subject)
+            self.assertEqual(ticket1.description, test_email_body)
 
             ticket2 = get_object_or_404(Ticket, pk=2)
             self.assertEqual(ticket2.ticket_for_url, "QQ-%s" % ticket2.id)
-            self.assertEqual(ticket2.description, "This is the helpdesk comment via email.")
+            self.assertEqual(ticket2.title, test_email_subject)
+            self.assertEqual(ticket2.description, test_email_body)
+
+    def test_read_html_multipart_email(self):
+        """Tests reading multipart MIME (HTML body and plain text alternative)
+           emails from a queue and creating tickets.
+           For each email source supported, we mock the backend to provide
+           authentically formatted responses containing our test data."""
+
+        # example email text from Python docs: https://docs.python.org/3/library/email-examples.html
+        from email.mime.multipart import MIMEMultipart
+        from email.mime.text import MIMEText
+
+        me = "my@example.com"
+        you = "your@example.com"
+        subject = "Link"
+
+        # Create message container - the correct MIME type is multipart/alternative.
+        msg = MIMEMultipart('alternative')
+        msg['Subject'] = subject
+        msg['From'] = me
+        msg['To'] = you
+
+        # Create the body of the message (a plain-text and an HTML version).
+        text = "Hi!\nHow are you?\nHere is the link you wanted:\nhttps://www.python.org"
+        html = """\
+        <html>
+        <head></head>
+        <body>
+            <p>Hi!<br>
+            How are you?<br>
+            Here is the <a href="https://www.python.org">link</a> you wanted.
+            </p>
+        </body>
+        </html>
+        """
+
+        # Record the MIME types of both parts - text/plain and text/html.
+        part1 = MIMEText(text, 'plain')
+        part2 = MIMEText(html, 'html')
+
+        # Attach parts into message container.
+        # According to RFC 2046, the last part of a multipart message, in this case
+        # the HTML message, is best and preferred.
+        msg.attach(part1)
+        msg.attach(part2)
+
+        test_mail_len = len(msg)
+
+        if self.socks:
+            from socks import ProxyConnectionError
+            with self.assertRaisesRegexp(ProxyConnectionError, '%s:%s' % (unrouted_socks_server, unused_port)):
+                call_command('get_email')
+
+        else:
+            # Test local email reading
+            if self.method == 'local':
+                with mock.patch('helpdesk.management.commands.get_email.listdir') as mocked_listdir, \
+                        mock.patch('helpdesk.management.commands.get_email.isfile') as mocked_isfile, \
+                        mock.patch('builtins.open' if six.PY3 else '__builtin__.open', mock.mock_open(read_data=msg.as_string())):
+                    mocked_isfile.return_value = True
+                    mocked_listdir.return_value = ['filename1', 'filename2']
+
+                    call_command('get_email')
+
+                    mocked_listdir.assert_called_with('/var/lib/mail/helpdesk/')
+                    mocked_isfile.assert_any_call('/var/lib/mail/helpdesk/filename1')
+                    mocked_isfile.assert_any_call('/var/lib/mail/helpdesk/filename2')
+
+            elif self.method == 'pop3':
+                # mock poplib.POP3's list and retr methods to provide responses as per RFC 1939
+                pop3_emails = {
+                    '1': ("+OK", msg.as_string().split('\n')),
+                    '2': ("+OK", msg.as_string().split('\n')),
+                }
+                pop3_mail_list = ("+OK 2 messages", ("1 %d" % test_mail_len, "2 %d" % test_mail_len))
+                mocked_poplib_server = mock.Mock()
+                mocked_poplib_server.list = mock.Mock(return_value=pop3_mail_list)
+                mocked_poplib_server.retr = mock.Mock(side_effect=lambda x: pop3_emails[x])
+                with mock.patch('helpdesk.management.commands.get_email.poplib', autospec=True) as mocked_poplib:
+                    mocked_poplib.POP3 = mock.Mock(return_value=mocked_poplib_server)
+                    call_command('get_email')
+
+            elif self.method == 'imap':
+                # mock imaplib.IMAP4's search and fetch methods with responses from RFC 3501
+                imap_emails = {
+                    "1": ("OK", (("1", msg.as_string()),)),
+                    "2": ("OK", (("2", msg.as_string()),)),
+                }
+                imap_mail_list = ("OK", ("1 2",))
+                mocked_imaplib_server = mock.Mock()
+                mocked_imaplib_server.search = mock.Mock(return_value=imap_mail_list)
+
+                # we ignore the second arg as the data item/mime-part is constant (RFC822)
+                mocked_imaplib_server.fetch = mock.Mock(side_effect=lambda x, _: imap_emails[x])
+                with mock.patch('helpdesk.management.commands.get_email.imaplib', autospec=True) as mocked_imaplib:
+                    mocked_imaplib.IMAP4 = mock.Mock(return_value=mocked_imaplib_server)
+                    call_command('get_email')
+
+            ticket1 = get_object_or_404(Ticket, pk=1)
+            self.assertEqual(ticket1.ticket_for_url, "QQ-%s" % ticket1.id)
+            self.assertEqual(ticket1.title, subject)
+            # plain text should become description
+            self.assertEqual(ticket1.description, text)
+            # HTML MIME part should be attached to follow up
+            followup1 = get_object_or_404(FollowUp, pk=1)
+            self.assertEqual(followup1.ticket.id, 1)
+            attach1 = get_object_or_404(Attachment, pk=1)
+            self.assertEqual(attach1.followup.id, 1)
+            self.assertEqual(attach1.filename, 'email_html_body.html')
+
+            ticket2 = get_object_or_404(Ticket, pk=2)
+            self.assertEqual(ticket2.ticket_for_url, "QQ-%s" % ticket2.id)
+            self.assertEqual(ticket2.title, subject)
+            # plain text should become description
+            self.assertEqual(ticket2.description, text)
+            # HTML MIME part should be attached to follow up
+            followup2 = get_object_or_404(FollowUp, pk=2)
+            self.assertEqual(followup2.ticket.id, 2)
+            attach2 = get_object_or_404(Attachment, pk=2)
+            self.assertEqual(attach2.followup.id, 2)
+            self.assertEqual(attach2.filename, 'email_html_body.html')
 
 
 # build matrix of test cases