From 4e001c7f15dd266ffb1b0dc7e1aebaca07c63642 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Fri, 2 Sep 2022 22:11:19 +0100 Subject: [PATCH 01/25] test utility methods to simplify more complex tests --- helpdesk/tests/utils.py | 122 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 helpdesk/tests/utils.py diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py new file mode 100644 index 00000000..2e36f04d --- /dev/null +++ b/helpdesk/tests/utils.py @@ -0,0 +1,122 @@ +"""UItility functions facilitate making unit testing easier and less brittle.""" + +import factory +import faker +import random +import re +import string +import unicodedata +from io import BytesIO +from email.message import Message +from email.mime.text import MIMEText +from numpy.random import randint +from PIL import Image +from typing import Tuple, Any + + +def strip_accents(text): + """ + Strip accents from input String. (only works on Pythin 3 + + :param text: The input string. + :type text: String. + + :returns: The processed String. + :rtype: String. + """ + text = unicodedata.normalize('NFD', text) + text = text.encode('ascii', 'ignore') + text = text.decode("utf-8") + return str(text) + + +def text_to_id(text): + """ + Convert input text to id. + + :param text: The input string. + :type text: String. + + :returns: The processed String. + :rtype: String. + """ + text = strip_accents(text.lower()) + text = re.sub('[ ]+', '_', text) + text = re.sub('[^0-9a-zA-Z_-]', '', text) + return text + + +def get_random_string(length: int=16) -> str: + return "".join( + [random.choice(string.ascii_letters + string.digits) for n in range(length)] + ) + + +def generate_random_image(image_format, array_dims): + """ + Creates an image from a random array. + + :param image_format: An image format (PNG or JPEG). + :param array_dims: A tuple with array dimensions. + + :returns: A byte string with encoded image + :rtype: bytes + """ + image_bytes = randint(low=0, high=255, size=array_dims, dtype='uint8') + io = BytesIO() + image_pil = Image.fromarray(image_bytes) + image_pil.save(io, image_format, subsampling=0, quality=100) + return io.getvalue() + + +def get_random_image(image_format: str="PNG", size: int=5): + """ + Returns a random image. + + Args: + image_format: An image format (PNG or JPEG). + + Returns: + A string with encoded image + """ + return generate_random_image(image_format, (size, size, 3)) + + +def get_fake(provider: str, locale: str = "en_US", min_length: int = 5) -> Any: + """ + Generates a random string, float, integer etc based on provider + Provider can be "text', 'sentence', + e.g. `get_fake('name')` ==> 'Buzz Aldrin' + """ + return factory.Faker(provider).evaluate({}, None, {'locale': locale,}) + + +def generate_email_address(locale: str="en_US") -> Tuple[str, str, str]: + """ + Generate an email address making sure that the email address itself contains only ascii + """ + fake = faker.Faker(locale=locale) + first_name = fake.first_name() + last_name = fake.last_name() + first_name.replace(' ', '').encode("ascii", "ignore").lower() + email_address = "{}.{}@{}".format( + first_name.replace(' ', '').encode("ascii", "ignore").lower().decode(), + last_name.replace(' ', '').encode("ascii", "ignore").lower().decode(), + get_random_string(5) + fake.domain_name() + ) + return email_address, first_name, last_name + + +def generate_email(locale: str="en_US", content_type: str="text/html", use_short_email: bool=False) -> Message: + """ + Generates an email includng headers + """ + to_meta = generate_email_address(locale) + from_meta = generate_email_address(locale) + body = get_fake("text", locale=locale) + + msg = MIMEText(body) + msg['Subject'] = get_fake("sentence", locale=locale) + msg['From'] = from_meta[0] if use_short_email else "{} {}<{}>".format(from_meta[1], from_meta[2], from_meta[0]) + msg['To'] = to_meta[0] if use_short_email else "{} {}<{}>".format(to_meta[1], to_meta[2], to_meta[0]) + return msg.as_string() From 3c4c9ce533a86c6b8d9e6cbadd38381a39b4f0d7 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Fri, 2 Sep 2022 22:12:42 +0100 Subject: [PATCH 02/25] Additional test libraries for more detailed testing of emails --- requirements-testing.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/requirements-testing.txt b/requirements-testing.txt index 12adeae7..80872424 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -7,3 +7,6 @@ pbr mock freezegun isort +numpy +factory_boy +faker \ No newline at end of file From e1085cb370fd2b31a4a868c465a69da771acb317 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Sun, 4 Sep 2022 21:53:08 +0100 Subject: [PATCH 03/25] Custom exception to handle ignored messages explicitly. --- helpdesk/exceptions.py | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 helpdesk/exceptions.py diff --git a/helpdesk/exceptions.py b/helpdesk/exceptions.py new file mode 100644 index 00000000..de35b9c4 --- /dev/null +++ b/helpdesk/exceptions.py @@ -0,0 +1,5 @@ +class IgnoreTicketException(Exception): + """ + Raised when an email message is received from a sender who is marked to be ignored + """ + pass \ No newline at end of file From 23c3b72a432a24f8f374881ffa80ffab57d5120f Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Sun, 4 Sep 2022 22:01:32 +0100 Subject: [PATCH 04/25] Refactor object_from_message to make it more testable and fix some bugs. Extract the from email using the email library instead of pre-decoding which exposes the comma separator causing the email address to be extracted incorrectly if the real name contains a comma. Raise an exception when a message to be ignored is detected to process ignored messages explicitly. --- helpdesk/email.py | 253 ++++++++++++++++++++++++---------------------- 1 file changed, 132 insertions(+), 121 deletions(-) diff --git a/helpdesk/email.py b/helpdesk/email.py index 1a0b7d50..7a4962e2 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -20,6 +20,7 @@ import email from email.utils import getaddresses from email_reply_parser import EmailReplyParser from helpdesk import settings +from helpdesk.exceptions import IgnoreTicketException from helpdesk.lib import process_attachments, safe_template_context from helpdesk.models import FollowUp, IgnoreEmail, Queue, Ticket import imaplib @@ -34,6 +35,8 @@ import ssl import sys from time import ctime import typing +from email.message import Message +from typing import Tuple, List # import User model, which may be a custom model @@ -135,8 +138,11 @@ def pop3_sync(q, logger, server): else: full_message = encoding.force_str( "\n".join(raw_content), errors='replace') - ticket = object_from_message( - message=full_message, queue=q, logger=logger) + try: + ticket = object_from_message(message=full_message, queue=q, logger=logger) + except IgnoreTicketException: + logger.warn( + "Message %s was ignored and will be left on POP3 server" % msgNum) if ticket: server.dele(msgNum) @@ -186,9 +192,12 @@ def imap_sync(q, logger, server): data = server.fetch(num, '(RFC822)')[1] full_message = encoding.force_str(data[0][1], errors='replace') try: - ticket = object_from_message( - message=full_message, queue=q, logger=logger) - except TypeError: + ticket = object_from_message(message=full_message, queue=q, logger=logger) + except IgnoreTicketException: + logger.warn("Message %s was ignored and will be left on IMAP server" % num) + return + except TypeError as te: + logger.warn(f"Unexpected error processing message: {te}") ticket = None # hotfix. Need to work out WHY. if ticket: server.store(num, '+FLAGS', '\\Deleted') @@ -282,8 +291,11 @@ def process_queue(q, logger): logger.info("Processing message %d" % i) with open(m, 'r') as f: full_message = encoding.force_str(f.read(), errors='replace') - ticket = object_from_message( - message=full_message, queue=q, logger=logger) + try: + ticket = object_from_message(message=full_message, queue=q, logger=logger) + except IgnoreTicketException: + logger.warn("Message %d was ignored and will be left in local directory", i) + return if ticket: logger.info( "Successfully processed message %d, ticket/comment created.", i) @@ -573,38 +585,122 @@ def get_email_body_from_part_payload(part) -> str: part.get_payload(decode=False) ) +def attempt_body_extract_from_html(message: str) -> str: + mail = BeautifulSoup(str(message), "html.parser") + beautiful_body = mail.find('body') + body = None + full_body = None + if beautiful_body: + try: + body = beautiful_body.text + full_body = body + except AttributeError: + pass + if not body: + body = "" + return body, full_body + +def extract_part_data( + part: Message, + counter: int, + ticket_id: int, + files: List, + logger: logging.Logger + ) -> Tuple[str, str]: + name = part.get_filename() + if name: + name = email.utils.collapse_rfc2231_value(name) + part_body = None + part_full_body=None + if part.get_content_maintype() == 'text' and name is None: + if part.get_content_subtype() == 'plain': + part_body = part.get_payload(decode=True) + # https://github.com/django-helpdesk/django-helpdesk/issues/732 + if part['Content-Transfer-Encoding'] == '8bit' and part.get_content_charset() == 'utf-8': + part_body = part_body.decode('unicode_escape') + part_body = decodeUnknown(part.get_content_charset(), part_body) + # have to use django_settings here so overwritting it works in tests + # the default value is False anyway + if ticket_id is None and getattr(django_settings, 'HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL', False): + # first message in thread, we save full body to avoid + # losing forwards and things like that + part_full_body = get_body_from_fragments(part_body) + part_body = EmailReplyParser.parse_reply(part_body) + else: + # second and other reply, save only first part of the + # message + part_body = EmailReplyParser.parse_reply(part_body) + part_full_body = part_body + # workaround to get unicode text out rather than escaped text + part_body = get_encoded_body(part_body) + logger.debug("Discovered plain text MIME part") + else: + email_body = get_email_body_from_part_payload(part) + + if not part_body and not part_full_body: + # no text has been parsed so far - try such deep parsing + # for some messages + altered_body = email_body.replace( + "

", "

\n").replace("{email_body}" + + payload = ( + '' + '' + '' + '' + '%s' + '' + ) % email_body + files.append( + SimpleUploadedFile( + _("email_html_body.html"), payload.encode("utf-8"), 'text/html') + ) + logger.debug("Discovered HTML MIME part") + else: + if not name: + ext = mimetypes.guess_extension(part.get_content_type()) + name = "part-%i%s" % (counter, ext) + else: + name = ("part-%i_" % counter) + name + + files.append(SimpleUploadedFile(name, part.get_payload(decode=True), mimetypes.guess_type(name)[0])) + logger.debug("Found MIME attachment %s" % name) + return part_body, part_full_body def object_from_message(message: str, queue: Queue, logger: logging.Logger ) -> Ticket: - # 'message' must be an RFC822 formatted message. - message = email.message_from_string(message) + # 'message' must be an RFC822 formatted message to correctly parse. + message_obj = email.message_from_string(message) - subject = message.get('subject', _('Comment from e-mail')) + subject = message_obj.get('subject', _('Comment from e-mail')) subject = decode_mail_headers( - decodeUnknown(message.get_charset(), subject)) + decodeUnknown(message_obj.get_charset(), subject)) for affix in STRIPPED_SUBJECT_STRINGS: subject = subject.replace(affix, "") subject = subject.strip() - sender = message.get('from', _('Unknown Sender')) - sender = decode_mail_headers(decodeUnknown(message.get_charset(), sender)) - - # to address bug #832, we wrap all the text in front of the email address in - # double quotes by using replace() on the email string. Then, - # take first item of list, second item of tuple is the actual email address. - # Note that the replace won't work on just an email with no real name, - # but the getaddresses() function seems to be able to handle just unclosed quotes - # correctly. Not ideal, but this seems to work for now. - sender_email = email.utils.getaddresses( - ['\"' + sender.replace('<', '\" <')])[0][1] + # TODO: Should really be assigning a properly formatted fake email. + # Check if anything relies on this being a "real name" formatted string if no sender is found on message_obj. + # Also not sure it should be accepting emails from unknown senders + sender_email = _('Unknown Sender') + sender_hdr = message_obj.get('from') + if sender_hdr: + # Parse the header which extracts the first email address in the list if more than one + # The parseaddr method returns a tuple in the form + # Only need the actual email address from the tuple not the "real name" + # Since the spec requires that all email addresses are ASCII, they will not be encoded + sender_email = email.utils.parseaddr(sender_hdr)[1] for ignore in IgnoreEmail.objects.filter(Q(queues=queue) | Q(queues__isnull=True)): if ignore.test(sender_email): - # By returning 'False' the message will be kept in the mailbox, - # and the 'True' will cause the message to be deleted. - return not ignore.keep_in_mailbox + raise IgnoreTicketException() ticket_id: typing.Optional[int] = get_ticket_id_from_subject_slug( queue.slug, @@ -617,108 +713,23 @@ def object_from_message(message: str, counter = 0 files = [] - for part in message.walk(): + for part in message_obj.walk(): if part.get_content_maintype() == 'multipart': continue - - name = part.get_param("name") - if name: - name = email.utils.collapse_rfc2231_value(name) - - if part.get_content_maintype() == 'text' and name is None: - if part.get_content_subtype() == 'plain': - body = part.get_payload(decode=True) - # https://github.com/django-helpdesk/django-helpdesk/issues/732 - if part['Content-Transfer-Encoding'] == '8bit' and part.get_content_charset() == 'utf-8': - body = body.decode('unicode_escape') - body = decodeUnknown(part.get_content_charset(), body) - # have to use django_settings here so overwritting it works in tests - # the default value is False anyway - if ticket_id is None and getattr(django_settings, 'HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL', False): - # first message in thread, we save full body to avoid - # losing forwards and things like that - full_body = get_body_from_fragments(body) - body = EmailReplyParser.parse_reply(body) - else: - # second and other reply, save only first part of the - # message - body = EmailReplyParser.parse_reply(body) - full_body = body - # workaround to get unicode text out rather than escaped text - body = get_encoded_body(body) - logger.debug("Discovered plain text MIME part") - else: - email_body = get_email_body_from_part_payload(part) - - if not body and not full_body: - # no text has been parsed so far - try such deep parsing - # for some messages - altered_body = email_body.replace( - "

", "

\n").replace("{email_body}" - - payload = ( - '' - '' - '' - '' - '%s' - '' - ) % email_body - files.append( - SimpleUploadedFile( - _("email_html_body.html"), payload.encode("utf-8"), 'text/html') - ) - logger.debug("Discovered HTML MIME part") - else: - if not name: - ext = mimetypes.guess_extension(part.get_content_type()) - name = "part-%i%s" % (counter, ext) - else: - name = ("part-%i_" % counter) + name - - # # FIXME: this code gets the paylods, then does something with it and then completely ignores it - # # writing the part.get_payload(decode=True) instead; and then the payload variable is - # # replaced by some dict later. - # # the `payloadToWrite` has been also ignored so was commented - # payload = part.get_payload() - # if isinstance(payload, list): - # payload = payload.pop().as_string() - # # payloadToWrite = payload - # # check version of python to ensure use of only the correct error type - # non_b64_err = TypeError - # try: - # logger.debug("Try to base64 decode the attachment payload") - # # payloadToWrite = base64.decodebytes(payload) - # except non_b64_err: - # logger.debug("Payload was not base64 encoded, using raw bytes") - # # payloadToWrite = payload - files.append(SimpleUploadedFile(name, part.get_payload( - decode=True), mimetypes.guess_type(name)[0])) - logger.debug("Found MIME attachment %s" % name) - + # See email.message_obj.Message.get_filename() + part_body, part_full_body = extract_part_data(part, counter, ticket_id, files, logger) + if part_body: + body = part_body + full_body = part_full_body counter += 1 if not body: - mail = BeautifulSoup(str(message), "html.parser") - beautiful_body = mail.find('body') - if beautiful_body: - try: - body = beautiful_body.text - full_body = body - except AttributeError: - pass - if not body: - body = "" + body, full_body = attempt_body_extract_from_html(message_obj) - add_file_if_always_save_incoming_email_message(files, message) + add_file_if_always_save_incoming_email_message(files, message_obj) - smtp_priority = message.get('priority', '') - smtp_importance = message.get('importance', '') + smtp_priority = message_obj.get('priority', '') + smtp_importance = message_obj.get('importance', '') high_priority_types = {'high', 'important', '1', 'urgent'} priority = 2 if high_priority_types & { smtp_priority, smtp_importance} else 3 @@ -733,4 +744,4 @@ def object_from_message(message: str, 'files': files, } - return create_object_from_email_message(message, ticket_id, payload, files, logger=logger) + return create_object_from_email_message(message_obj, ticket_id, payload, files, logger=logger) From 572ffd5acf09bd664c26ab868e63fc0abf0e5971 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Tue, 6 Sep 2022 19:40:35 +0100 Subject: [PATCH 05/25] Handle ignored emails explicitly using exceptions. Support the flag on IgnoreEmail model to control deleting the email if ignored. --- helpdesk/email.py | 70 ++++++++++++++++++-------------- helpdesk/exceptions.py | 8 ++++ helpdesk/tests/test_get_email.py | 42 +++++++++++++++---- helpdesk/tests/utils.py | 49 ++++++++++++++-------- 4 files changed, 115 insertions(+), 54 deletions(-) diff --git a/helpdesk/email.py b/helpdesk/email.py index 7a4962e2..7c83f7db 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -20,7 +20,7 @@ import email from email.utils import getaddresses from email_reply_parser import EmailReplyParser from helpdesk import settings -from helpdesk.exceptions import IgnoreTicketException +from helpdesk.exceptions import IgnoreTicketException, DeleteIgnoredTicketException from helpdesk.lib import process_attachments, safe_template_context from helpdesk.models import FollowUp, IgnoreEmail, Queue, Ticket import imaplib @@ -143,14 +143,18 @@ def pop3_sync(q, logger, server): except IgnoreTicketException: logger.warn( "Message %s was ignored and will be left on POP3 server" % msgNum) - - if ticket: - server.dele(msgNum) - logger.info( - "Successfully processed message %s, deleted from POP3 server" % msgNum) - else: + except DeleteIgnoredTicketException: logger.warn( - "Message %s was not successfully processed, and will be left on POP3 server" % msgNum) + "Message %s was ignored and deleted from POP3 server" % msgNum) + server.dele(msgNum) + else: + if ticket: + server.dele(msgNum) + logger.info( + "Successfully processed message %s, deleted from POP3 server" % msgNum) + else: + logger.warn( + "Message %s was not successfully processed, and will be left on POP3 server" % msgNum) server.quit() @@ -195,16 +199,19 @@ def imap_sync(q, logger, server): ticket = object_from_message(message=full_message, queue=q, logger=logger) except IgnoreTicketException: logger.warn("Message %s was ignored and will be left on IMAP server" % num) - return - except TypeError as te: - logger.warn(f"Unexpected error processing message: {te}") - ticket = None # hotfix. Need to work out WHY. - if ticket: + except DeleteIgnoredTicketException: server.store(num, '+FLAGS', '\\Deleted') - logger.info( - "Successfully processed message %s, deleted from IMAP server" % num) + logger.warn("Message %s was ignored and deleted from IMAP server" % num) + except TypeError as te: + # Log the error with stacktrace to help identify what went wrong + logger.error(f"Unexpected error processing message: {te}", exc_info=True) else: - logger.warn( + if ticket: + server.store(num, '+FLAGS', '\\Deleted') + logger.info( + "Successfully processed message %s, deleted from IMAP server" % num) + else: + logger.warn( "Message %s was not successfully processed, and will be left on IMAP server" % num) except imaplib.IMAP4.error: logger.error( @@ -295,21 +302,24 @@ def process_queue(q, logger): ticket = object_from_message(message=full_message, queue=q, logger=logger) except IgnoreTicketException: logger.warn("Message %d was ignored and will be left in local directory", i) - return - if ticket: - logger.info( - "Successfully processed message %d, ticket/comment created.", i) - try: - # delete message file if ticket was successful + except DeleteIgnoredTicketException: os.unlink(m) - except OSError as e: - logger.error( - "Unable to delete message %d (%s).", i, str(e)) + logger.warn("Message %d was ignored and deleted local directory", i) else: - logger.info("Successfully deleted message %d.", i) - else: - logger.warn( - "Message %d was not successfully processed, and will be left in local directory", i) + if ticket: + logger.info( + "Successfully processed message %d, ticket/comment created.", i) + try: + # delete message file if ticket was successful + os.unlink(m) + except OSError as e: + logger.error( + "Unable to delete message %d (%s).", i, str(e)) + else: + logger.info("Successfully deleted message %d.", i) + else: + logger.warn( + "Message %d was not successfully processed, and will be left in local directory", i) def decodeUnknown(charset, string): @@ -700,7 +710,7 @@ def object_from_message(message: str, for ignore in IgnoreEmail.objects.filter(Q(queues=queue) | Q(queues__isnull=True)): if ignore.test(sender_email): - raise IgnoreTicketException() + raise IgnoreTicketException() if ignore.keep_in_mailbox else DeleteIgnoredTicketException() ticket_id: typing.Optional[int] = get_ticket_id_from_subject_slug( queue.slug, diff --git a/helpdesk/exceptions.py b/helpdesk/exceptions.py index de35b9c4..d87f74fd 100644 --- a/helpdesk/exceptions.py +++ b/helpdesk/exceptions.py @@ -2,4 +2,12 @@ class IgnoreTicketException(Exception): """ Raised when an email message is received from a sender who is marked to be ignored """ + pass + + +class DeleteIgnoredTicketException(Exception): + """ + Raised when an email message is received from a sender who is marked to be ignored + and the record is tagged to delete the email from the inbox + """ pass \ No newline at end of file diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index 89afa95d..df59fe80 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -7,7 +7,8 @@ from django.shortcuts import get_object_or_404 from django.test import override_settings, TestCase import helpdesk.email from helpdesk.management.commands.get_email import Command -from helpdesk.models import FollowUp, FollowUpAttachment, Queue, Ticket, TicketCC +from helpdesk.models import FollowUp, FollowUpAttachment, Queue, Ticket, TicketCC,\ + IgnoreEmail import itertools import logging import os @@ -16,6 +17,9 @@ import six import sys from tempfile import mkdtemp from unittest import mock +from helpdesk.tests import utils +from helpdesk.exceptions import DeleteIgnoredTicketException, IgnoreTicketException +from helpdesk.email import object_from_message THIS_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -42,7 +46,7 @@ class GetEmailCommonTests(TestCase): with mock.patch.object(Command, 'handle', return_value=None) as mocked_handle: call_command('get_email', quiet=quiet_test_value) mocked_handle.assert_called_once() - for args, kwargs in mocked_handle.call_args_list: + for _, kwargs in mocked_handle.call_args_list: self.assertEqual(quiet_test_value, (kwargs['quiet'])) def test_email_with_blank_body_and_attachment(self): @@ -132,7 +136,29 @@ class GetEmailCommonTests(TestCase): assert "Hello there!" in FollowUp.objects.filter( ticket=ticket).first().comment + def test_will_delete_ignored_email(self): + """ + Tests if an email will be ignored if configured to do so and throws the correct exception + to ensure the email is deleted + """ + message, from_meta, _ = utils.generate_text_email(locale="es_ES") + ignore = IgnoreEmail(name="Test Ignore", email_address=from_meta[1], keep_in_mailbox=False) + ignore.save() + with self.assertRaises(DeleteIgnoredTicketException): + object_from_message(message.as_string(), self.queue_public, self.logger); + def test_will_not_delete_ignored_email(self): + """ + Tests if an email will be ignored if configured to do so and throws the correct exception + to ensure the email is NOT deleted + """ + message, from_meta, _ = utils.generate_text_email(locale="es_ES") + ignore = IgnoreEmail(name="Test Ignore", email_address=from_meta[1], keep_in_mailbox=True) + ignore.save() + with self.assertRaises(IgnoreTicketException): + object_from_message(message.as_string(), self.queue_public, self.logger); + + class GetEmailParametricTemplate(object): """TestCase that checks basic email functionality across methods and socks configs.""" @@ -258,12 +284,12 @@ class GetEmailParametricTemplate(object): """Tests correctly decoding mail headers when a comma is encoded into UTF-8. See bug report #832.""" - # example email text from Django docs: - # https://docs.djangoproject.com/en/1.10/ref/unicode/ - test_email_from = "Bernard-Bouissières, Benjamin " + # Create the from using standard RFC required formats + # Override the last_name to ensure we get a non-ascii character in it + test_email_from_meta = utils.generate_email_address("fr_FR", last_name_override="Bouissières") test_email_subject = "Commas in From lines" test_email_body = "Testing commas in from email UTF-8." - test_email = "To: helpdesk@example.com\nFrom: " + test_email_from + \ + test_email = "To: helpdesk@example.com\nFrom: " + test_email_from_meta[0] + \ "\nSubject: " + test_email_subject + "\n\n" + test_email_body test_mail_len = len(test_email) @@ -333,13 +359,13 @@ class GetEmailParametricTemplate(object): ticket1 = get_object_or_404(Ticket, pk=1) self.assertEqual(ticket1.ticket_for_url, "QQ-%s" % ticket1.id) - self.assertEqual(ticket1.submitter_email, 'bbb@example.com') + self.assertEqual(ticket1.submitter_email, test_email_from_meta[1]) 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.submitter_email, 'bbb@example.com') + self.assertEqual(ticket2.submitter_email, test_email_from_meta[1]) self.assertEqual(ticket2.title, test_email_subject) self.assertEqual(ticket2.description, test_email_body) diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py index 2e36f04d..2cd4dd50 100644 --- a/helpdesk/tests/utils.py +++ b/helpdesk/tests/utils.py @@ -1,5 +1,6 @@ """UItility functions facilitate making unit testing easier and less brittle.""" +import email import factory import faker import random @@ -11,7 +12,8 @@ from email.message import Message from email.mime.text import MIMEText from numpy.random import randint from PIL import Image -from typing import Tuple, Any +from typing import Tuple, Any, Optional +import typing def strip_accents(text): @@ -48,7 +50,7 @@ def text_to_id(text): def get_random_string(length: int=16) -> str: return "".join( - [random.choice(string.ascii_letters + string.digits) for n in range(length)] + [random.choice(string.ascii_letters + string.digits) for _ in range(length)] ) @@ -91,32 +93,47 @@ def get_fake(provider: str, locale: str = "en_US", min_length: int = 5) -> Any: return factory.Faker(provider).evaluate({}, None, {'locale': locale,}) -def generate_email_address(locale: str="en_US") -> Tuple[str, str, str]: - """ - Generate an email address making sure that the email address itself contains only ascii - """ +def generate_email_address( + locale: str="en_US", + use_short_email: bool=False, + real_name_format: Optional[str]="{last_name}, {first_name}", + last_name_override: Optional[str]=None) -> Tuple[str, str, str, str]: + ''' + Generate an RFC 2822 email address + + :param locale: change this to generate locale specific names + :param use_short_email: defaults to false. If true then does not include real name in email address + :param real_name_format: pass a different format if different than "{last_name}, {first_name}" + :param last_name_override: override the fake name if you want some special characters in the last name + :returns , , , Message: +def generate_text_email(locale: str="en_US", + content_type: str="text/plain", + use_short_email: bool=False + ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: """ - Generates an email includng headers + Generates an email including headers """ - to_meta = generate_email_address(locale) - from_meta = generate_email_address(locale) + to_meta = generate_email_address(locale, use_short_email=use_short_email) + from_meta = generate_email_address(locale, use_short_email=use_short_email) body = get_fake("text", locale=locale) msg = MIMEText(body) msg['Subject'] = get_fake("sentence", locale=locale) - msg['From'] = from_meta[0] if use_short_email else "{} {}<{}>".format(from_meta[1], from_meta[2], from_meta[0]) - msg['To'] = to_meta[0] if use_short_email else "{} {}<{}>".format(to_meta[1], to_meta[2], to_meta[0]) - return msg.as_string() + msg['From'] = from_meta[0] + msg['To'] = to_meta[0] + return msg, from_meta, to_meta From 96b985f8a6daa58b61d58eb9fdf763cb3a23f580 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Tue, 6 Sep 2022 19:40:46 +0100 Subject: [PATCH 06/25] Ignore tox created stuff --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 162dbae9..5ff0d848 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ docs/doctrees/* .directory *.swp .idea +.tox/** # ignore demo attachments that user might have added helpdesk/attachments/ From 99b2ba53729150ae7e643a2d1c42394d51af1c10 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Tue, 6 Sep 2022 19:41:29 +0100 Subject: [PATCH 07/25] Remove the directory created for the test --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index e71c5e2a..f0863605 100644 --- a/Makefile +++ b/Makefile @@ -56,6 +56,7 @@ test: mkdir -p var $(PIP) install -e .[test] $(TOX) + rm -rf var #: documentation - Build documentation (Sphinx, README, ...). From 357241e269cd73df8f9fdfb29d1700954f4086ae Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Wed, 7 Sep 2022 08:48:32 +0100 Subject: [PATCH 08/25] Refactor to make the methods more generic and able to be used to build any kind of email --- helpdesk/tests/utils.py | 139 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 127 insertions(+), 12 deletions(-) diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py index 2cd4dd50..3ae7f843 100644 --- a/helpdesk/tests/utils.py +++ b/helpdesk/tests/utils.py @@ -6,14 +6,18 @@ import faker import random import re import string +import typing import unicodedata -from io import BytesIO +from email import encoders from email.message import Message +from email.mime.base import MIMEBase +from email.mime.image import MIMEImage from email.mime.text import MIMEText +from io import BytesIO from numpy.random import randint from PIL import Image from typing import Tuple, Any, Optional -import typing +from email.mime.multipart import MIMEMultipart def strip_accents(text): @@ -85,13 +89,29 @@ def get_random_image(image_format: str="PNG", size: int=5): def get_fake(provider: str, locale: str = "en_US", min_length: int = 5) -> Any: + """ + Generates a random string, float, integer etc based on provider + Provider can be "text', 'sentence', "word" + e.g. `get_fake('name')` ==> 'Buzz Aldrin' + """ + string = factory.Faker(provider).evaluate({}, None, {'locale': locale,}) + while len(string) < min_length: + string += factory.Faker(provider).evaluate({}, None, {'locale': locale,}) + return string + + +def get_fake_html(locale: str = "en_US", wrap_in_body_tag=True) -> Any: """ Generates a random string, float, integer etc based on provider Provider can be "text', 'sentence', e.g. `get_fake('name')` ==> 'Buzz Aldrin' """ - return factory.Faker(provider).evaluate({}, None, {'locale': locale,}) - + html = factory.Faker("sentence").evaluate({}, None, {'locale': locale,}) + for _ in range(0,4): + html += "
  • " + factory.Faker("sentence").evaluate({}, None, {'locale': locale,}) + "
    • " + for _ in range(0,4): + html += "

    " + factory.Faker("text").evaluate({}, None, {'locale': locale,}) + return f"{html}" if wrap_in_body_tag else html def generate_email_address( locale: str="en_US", @@ -120,20 +140,115 @@ def generate_email_address( # format email address for RFC 2822 and return return email.utils.formataddr((real_name, email_address)), email_address, first_name, last_name +def generate_file_mime_part(locale: str="en_US",filename: str = None) -> Message: + """ + + :param locale: change this to generate locale specific file name and attachment content + :param filename: pass a file name if you want t ospecify a specific name otherwise a random name will be generated + """ + part = MIMEBase('application', 'octet-stream') + part.set_payload(get_fake("text", locale=locale, min_length=1024)) + encoders.encode_base64(part) + if not filename: + filename = get_fake("word", locale=locale, min_length=8) + ".txt" + part.add_header('Content-Disposition', "attachment; filename= %s" % filename) + return part + +def generate_image_mime_part(locale: str="en_US",imagename: str = None) -> Message: + """ + + :param locale: change this to generate locale specific file name and attachment content + :param filename: pass a file name if you want t ospecify a specific name otherwise a random name will be generated + """ + part = MIMEImage(generate_random_image(image_format="JPEG")) + part.set_payload(get_fake("text", locale=locale, min_length=1024)) + encoders.encode_base64(part) + if not imagename: + imagename = get_fake("word", locale=locale, min_length=8) + ".jpg" + part.add_header('Content-Disposition', "attachment; filename= %s" % imagename) + return part + +def add_email_headers(message: Message, locale: str="en_US", + use_short_email: bool=False + ) -> typing.Tuple[typing.Tuple[str, str], typing.Tuple[str, str]]: + """ + Adds the key email headers to a Mime part + + :param message: the Mime part to add headers to + :param locale: change this to generate locale specific "real names" and subject + :param use_short_email: produces a "To" or "From" that is only the email address if True + + """ + to_meta = generate_email_address(locale, use_short_email=use_short_email) + from_meta = generate_email_address(locale, use_short_email=use_short_email) + + message['Subject'] = get_fake("sentence", locale=locale) + message['From'] = from_meta[0] + message['To'] = to_meta[0] + return from_meta, to_meta + +def generate_mime_part(locale: str="en_US", + part_type: str="plain", + use_short_email: bool=False + ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: + """ + Generates amime part of the sepecified type + + :param locale: change this to generate locale specific strings + :param text_type: options are plain, html, image (attachment), file (attachment) + :param use_short_email: produces a "To" or "From" that is only the email address if True + """ + if "plain" == part_type: + body = get_fake("text", locale=locale, min_length=1024) + msg = MIMEText(body) + elif "html" == part_type: + body = get_fake_html(locale=locale, wrap_in_body_tag=True) + msg = MIMEText(body) + elif "file" == part_type: + msg = generate_file_mime_part(locale=locale) + msg = MIMEText(body) + elif "image" == part_type: + msg = generate_image_mime_part(locale=locale) + msg = MIMEText(body) + else: + raise Exception("Mime part not implemented: " + part_type) + return msg + +def generate_multipart_email(locale: str="en_US", + type_list: typing.List[str]=["plain", "html", "attachment"], + use_short_email: bool=False + ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: + """ + Generates an email including headers with the defined multiparts + + :param locale: + :param type_list: options are plain, html, image (attachment), file (attachment) + :param use_short_email: produces a "To" or "From" that is only the email address if True + """ + msg = MIMEMultipart() + for part_type in type_list: + msg.append(generate_mime_part(locale=locale, part_type=part_type, use_short_email=use_short_email)) + from_meta, to_meta = add_email_headers(msg, locale=locale, use_short_email=use_short_email) + return msg, from_meta, to_meta def generate_text_email(locale: str="en_US", - content_type: str="text/plain", use_short_email: bool=False ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: """ Generates an email including headers """ - to_meta = generate_email_address(locale, use_short_email=use_short_email) - from_meta = generate_email_address(locale, use_short_email=use_short_email) - body = get_fake("text", locale=locale) - + body = get_fake("text", locale=locale, min_length=1024) msg = MIMEText(body) - msg['Subject'] = get_fake("sentence", locale=locale) - msg['From'] = from_meta[0] - msg['To'] = to_meta[0] + from_meta, to_meta = add_email_headers(msg, locale=locale, use_short_email=use_short_email) + return msg, from_meta, to_meta + +def generate_html_email(locale: str="en_US", + use_short_email: bool=False + ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: + """ + Generates an email including headers + """ + body = get_fake_html(locale=locale) + msg = MIMEText(body) + from_meta, to_meta = add_email_headers(msg, locale=locale, use_short_email=use_short_email) return msg, from_meta, to_meta From eca2255f56f85b3c8c60bfa42822bf013ed08d18 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Thu, 8 Sep 2022 10:05:09 +0100 Subject: [PATCH 09/25] Test for filename using diaretics --- helpdesk/email.py | 10 +++++----- helpdesk/tests/test_get_email.py | 16 +++++++++++++++- helpdesk/tests/utils.py | 27 +++++++++++++++++++++------ 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/helpdesk/email.py b/helpdesk/email.py index 7c83f7db..7e0de522 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -621,7 +621,7 @@ def extract_part_data( if name: name = email.utils.collapse_rfc2231_value(name) part_body = None - part_full_body=None + part_full_body = None if part.get_content_maintype() == 'text' and name is None: if part.get_content_subtype() == 'plain': part_body = part.get_payload(decode=True) @@ -629,7 +629,7 @@ def extract_part_data( if part['Content-Transfer-Encoding'] == '8bit' and part.get_content_charset() == 'utf-8': part_body = part_body.decode('unicode_escape') part_body = decodeUnknown(part.get_content_charset(), part_body) - # have to use django_settings here so overwritting it works in tests + # have to use django_settings here so overwriting it works in tests # the default value is False anyway if ticket_id is None and getattr(django_settings, 'HELPDESK_FULL_FIRST_MESSAGE_FROM_EMAIL', False): # first message in thread, we save full body to avoid @@ -674,12 +674,12 @@ def extract_part_data( else: if not name: ext = mimetypes.guess_extension(part.get_content_type()) - name = "part-%i%s" % (counter, ext) + name = f"part-{counter}{ext}" else: - name = ("part-%i_" % counter) + name + name = f"part-{counter}_{name}" files.append(SimpleUploadedFile(name, part.get_payload(decode=True), mimetypes.guess_type(name)[0])) - logger.debug("Found MIME attachment %s" % name) + logger.debug("Found MIME attachment %s", name) return part_body, part_full_body def object_from_message(message: str, diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index df59fe80..14f8398e 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -19,7 +19,9 @@ from tempfile import mkdtemp from unittest import mock from helpdesk.tests import utils from helpdesk.exceptions import DeleteIgnoredTicketException, IgnoreTicketException -from helpdesk.email import object_from_message +from helpdesk.email import object_from_message, extract_part_data +from django.core.files.uploadedfile import SimpleUploadedFile +import typing THIS_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -158,6 +160,18 @@ class GetEmailCommonTests(TestCase): with self.assertRaises(IgnoreTicketException): object_from_message(message.as_string(), self.queue_public, self.logger); + def test_utf8_filename_attachment(self): + """ + Tests if an attachment correctly sent with a UTF8 filename in disposition is extracted correctly + """ + filename = "TeléfonoMañana.txt" + part = utils.generate_file_mime_part(locale="es_ES", filename=filename) + files: typing.List[SimpleUploadedFile] = [] + extract_part_data(part, counter=1, ticket_id="tst1", files=files, logger=self.logger) + sent_file: SimpleUploadedFile = files[0] + # The extractor prepends a part identifier so compare the ending + self.assertTrue(sent_file.name.endswith(filename), f"Filename extracted does not match: {sent_file.name}") + class GetEmailParametricTemplate(object): """TestCase that checks basic email functionality across methods and socks configs.""" diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py index 3ae7f843..38ddcd53 100644 --- a/helpdesk/tests/utils.py +++ b/helpdesk/tests/utils.py @@ -144,7 +144,7 @@ def generate_file_mime_part(locale: str="en_US",filename: str = None) -> Message """ :param locale: change this to generate locale specific file name and attachment content - :param filename: pass a file name if you want t ospecify a specific name otherwise a random name will be generated + :param filename: pass a file name if you want to specify a specific name otherwise a random name will be generated """ part = MIMEBase('application', 'octet-stream') part.set_payload(get_fake("text", locale=locale, min_length=1024)) @@ -158,7 +158,7 @@ def generate_image_mime_part(locale: str="en_US",imagename: str = None) -> Messa """ :param locale: change this to generate locale specific file name and attachment content - :param filename: pass a file name if you want t ospecify a specific name otherwise a random name will be generated + :param filename: pass a file name if you want to specify a specific name otherwise a random name will be generated """ part = MIMEImage(generate_random_image(image_format="JPEG")) part.set_payload(get_fake("text", locale=locale, min_length=1024)) @@ -168,7 +168,22 @@ def generate_image_mime_part(locale: str="en_US",imagename: str = None) -> Messa part.add_header('Content-Disposition', "attachment; filename= %s" % imagename) return part -def add_email_headers(message: Message, locale: str="en_US", +def generate_email_list(address_cnt: int = 3, + locale: str="en_US", + use_short_email: bool=False + ) -> str: + """ + Generates a list of email addresses formatted for email headers on a Mime part + + :param address_cnt: the number of email addresses to string together + :param locale: change this to generate locale specific "real names" and subject + :param use_short_email: produces a email address without "real name" if True + + """ + email_address_list = [generate_email_address(locale, use_short_email=use_short_email)[0] for _ in range(0, address_cnt)] + return ",".join(email_address_list) + +def add_simple_email_headers(message: Message, locale: str="en_US", use_short_email: bool=False ) -> typing.Tuple[typing.Tuple[str, str], typing.Tuple[str, str]]: """ @@ -228,7 +243,7 @@ def generate_multipart_email(locale: str="en_US", msg = MIMEMultipart() for part_type in type_list: msg.append(generate_mime_part(locale=locale, part_type=part_type, use_short_email=use_short_email)) - from_meta, to_meta = add_email_headers(msg, locale=locale, use_short_email=use_short_email) + from_meta, to_meta = add_simple_email_headers(msg, locale=locale, use_short_email=use_short_email) return msg, from_meta, to_meta def generate_text_email(locale: str="en_US", @@ -239,7 +254,7 @@ def generate_text_email(locale: str="en_US", """ body = get_fake("text", locale=locale, min_length=1024) msg = MIMEText(body) - from_meta, to_meta = add_email_headers(msg, locale=locale, use_short_email=use_short_email) + from_meta, to_meta = add_simple_email_headers(msg, locale=locale, use_short_email=use_short_email) return msg, from_meta, to_meta def generate_html_email(locale: str="en_US", @@ -250,5 +265,5 @@ def generate_html_email(locale: str="en_US", """ body = get_fake_html(locale=locale) msg = MIMEText(body) - from_meta, to_meta = add_email_headers(msg, locale=locale, use_short_email=use_short_email) + from_meta, to_meta = add_simple_email_headers(msg, locale=locale, use_short_email=use_short_email) return msg, from_meta, to_meta From b78be0d71f067ab7e5c070514642261906c7b16b Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Thu, 8 Sep 2022 22:11:07 +0100 Subject: [PATCH 10/25] Delete no longer used CI/CD --- .travis.yml | 26 -------------------------- 1 file changed, 26 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 3c8da6b2..00000000 --- a/.travis.yml +++ /dev/null @@ -1,26 +0,0 @@ -language: python -dist: focal # use LTS 20.04 - -python: - - "3.6" - - "3.7" - - "3.8" - -env: - - DJANGO=2.2.23 - - DJANGO=3.1.11 - - DJANGO=3.2.3 - -install: - - pip install -q Django==$DJANGO - - pip install -q -r requirements.txt - - pip install -q -r requirements-testing.txt - -before_script: - - "pycodestyle --exclude=migrations --ignore=E501,W503,W504 helpdesk" - -script: - - coverage run --source='.' quicktest.py helpdesk - -after_success: - - codecov From 8d7ba415fa234e32d422b54e175797ace2b78ac5 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Thu, 8 Sep 2022 22:11:24 +0100 Subject: [PATCH 11/25] Fix formatting per flake8 --- helpdesk/email.py | 33 ++++++++++++++++++--------------- helpdesk/exceptions.py | 2 +- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/helpdesk/email.py b/helpdesk/email.py index 7e0de522..c753aea5 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -212,7 +212,7 @@ def imap_sync(q, logger, server): "Successfully processed message %s, deleted from IMAP server" % num) else: logger.warn( - "Message %s was not successfully processed, and will be left on IMAP server" % num) + "Message %s was not successfully processed, and will be left on IMAP server" % num) except imaplib.IMAP4.error: logger.error( "IMAP retrieve failed. Is the folder '%s' spelled correctly, and does it exist on the server?", @@ -595,20 +595,22 @@ def get_email_body_from_part_payload(part) -> str: part.get_payload(decode=False) ) + def attempt_body_extract_from_html(message: str) -> str: - mail = BeautifulSoup(str(message), "html.parser") - beautiful_body = mail.find('body') - body = None - full_body = None - if beautiful_body: - try: - body = beautiful_body.text - full_body = body - except AttributeError: - pass - if not body: - body = "" - return body, full_body + mail = BeautifulSoup(str(message), "html.parser") + beautiful_body = mail.find('body') + body = None + full_body = None + if beautiful_body: + try: + body = beautiful_body.text + full_body = body + except AttributeError: + pass + if not body: + body = "" + return body, full_body + def extract_part_data( part: Message, @@ -616,7 +618,7 @@ def extract_part_data( ticket_id: int, files: List, logger: logging.Logger - ) -> Tuple[str, str]: +) -> Tuple[str, str]: name = part.get_filename() if name: name = email.utils.collapse_rfc2231_value(name) @@ -682,6 +684,7 @@ def extract_part_data( logger.debug("Found MIME attachment %s", name) return part_body, part_full_body + def object_from_message(message: str, queue: Queue, logger: logging.Logger diff --git a/helpdesk/exceptions.py b/helpdesk/exceptions.py index d87f74fd..4dbf59a5 100644 --- a/helpdesk/exceptions.py +++ b/helpdesk/exceptions.py @@ -10,4 +10,4 @@ class DeleteIgnoredTicketException(Exception): Raised when an email message is received from a sender who is marked to be ignored and the record is tagged to delete the email from the inbox """ - pass \ No newline at end of file + pass From 673d12c67be5dcf35433844e80d8a9e3f5c09881 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Thu, 8 Sep 2022 22:12:20 +0100 Subject: [PATCH 12/25] Add a dev requirements to make it easier to develop helpdesk --- requirements-dev.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 requirements-dev.txt diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 00000000..79669e95 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,4 @@ +tox +flake8 +pycodestyle +isort From 9627c17a455ef04700562640d2322b9528c0db77 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Thu, 8 Sep 2022 23:31:02 +0100 Subject: [PATCH 13/25] Enhance for development processes --- Makefile | 13 +++++++++++++ README.rst | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/Makefile b/Makefile index f0863605..33c39c74 100644 --- a/Makefile +++ b/Makefile @@ -57,6 +57,19 @@ test: $(PIP) install -e .[test] $(TOX) rm -rf var + + + +#: format - Run the PEP8 formatter. +.PHONY: format +format: + autopep8 --exit-code --global-config .flake8 helpdesk + + +#: checkformat - checks formatting against configured format specifications for the project. +.PHONY: checkformat +checkformat: + flake8 helpdesk --count --show-source --statistics --exit-zero --max-complexity=20 #: documentation - Build documentation (Sphinx, README, ...). diff --git a/README.rst b/README.rst index d7cb3237..9cfbd9a5 100644 --- a/README.rst +++ b/README.rst @@ -69,9 +69,30 @@ Django project. For further installation information see `docs/install.html` and `docs/configuration.html` +Developer Environment +--------------------- + +Follow these steps to set up your development environment to contribute to helpdesk: + - install a virtual environment + - using virtualenv from the helpdesk base folder do:: + virtualenv .venv && source .venv/bin/activate + + - install the requirements for development:: + pip install -r requirements.txt -r requirements-dev.txt + +To see option for the Makefile run: `make` + +The project enforces a standardized formatting in the CI/CD pipeline. To ensure you have the correct formatting run:: + make checkformat + +To auto format any code use this:: + make format + Testing ------- +From the command line you can run the tests using: `make test` + See `quicktest.py` for usage details. Upgrading from previous versions From 0ae288374caa7648efdf803ff62e20d8d7d3ac97 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Thu, 8 Sep 2022 23:31:28 +0100 Subject: [PATCH 14/25] Development support apps --- requirements-dev.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements-dev.txt b/requirements-dev.txt index 79669e95..2501f30c 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,4 +1,5 @@ tox +autopep8 flake8 pycodestyle isort From 9e3a3abcf1853a25cdaa171599a29a35a3fcab1c Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Thu, 8 Sep 2022 23:40:49 +0100 Subject: [PATCH 15/25] Fix import sorting --- helpdesk/email.py | 7 ++++--- helpdesk/tests/test_get_email.py | 14 +++++++------- helpdesk/tests/utils.py | 21 +++++++++++---------- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/helpdesk/email.py b/helpdesk/email.py index c753aea5..85dd07d4 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -7,6 +7,7 @@ See LICENSE for details. # import base64 + from bs4 import BeautifulSoup from datetime import timedelta from django.conf import settings as django_settings @@ -17,10 +18,11 @@ from django.db.models import Q from django.utils import encoding, timezone from django.utils.translation import gettext as _ import email +from email.message import Message from email.utils import getaddresses from email_reply_parser import EmailReplyParser from helpdesk import settings -from helpdesk.exceptions import IgnoreTicketException, DeleteIgnoredTicketException +from helpdesk.exceptions import DeleteIgnoredTicketException, IgnoreTicketException from helpdesk.lib import process_attachments, safe_template_context from helpdesk.models import FollowUp, IgnoreEmail, Queue, Ticket import imaplib @@ -35,8 +37,7 @@ import ssl import sys from time import ctime import typing -from email.message import Message -from typing import Tuple, List +from typing import List, Tuple # import User model, which may be a custom model diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index 14f8398e..a3ae7aee 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -1,14 +1,18 @@ # -*- coding: utf-8 -*- + from django.contrib.auth.hashers import make_password from django.contrib.auth.models import User +from django.core.files.uploadedfile import SimpleUploadedFile from django.core.management import call_command from django.shortcuts import get_object_or_404 from django.test import override_settings, TestCase import helpdesk.email +from helpdesk.email import extract_part_data, object_from_message +from helpdesk.exceptions import DeleteIgnoredTicketException, IgnoreTicketException from helpdesk.management.commands.get_email import Command -from helpdesk.models import FollowUp, FollowUpAttachment, Queue, Ticket, TicketCC,\ - IgnoreEmail +from helpdesk.models import FollowUp, FollowUpAttachment, IgnoreEmail, Queue, Ticket, TicketCC +from helpdesk.tests import utils import itertools import logging import os @@ -16,12 +20,8 @@ from shutil import rmtree import six import sys from tempfile import mkdtemp -from unittest import mock -from helpdesk.tests import utils -from helpdesk.exceptions import DeleteIgnoredTicketException, IgnoreTicketException -from helpdesk.email import object_from_message, extract_part_data -from django.core.files.uploadedfile import SimpleUploadedFile import typing +from unittest import mock THIS_DIR = os.path.dirname(os.path.abspath(__file__)) diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py index 38ddcd53..00b7e6a5 100644 --- a/helpdesk/tests/utils.py +++ b/helpdesk/tests/utils.py @@ -1,23 +1,24 @@ """UItility functions facilitate making unit testing easier and less brittle.""" + +from PIL import Image import email -import factory -import faker -import random -import re -import string -import typing -import unicodedata from email import encoders from email.message import Message from email.mime.base import MIMEBase from email.mime.image import MIMEImage +from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText +import factory +import faker from io import BytesIO from numpy.random import randint -from PIL import Image -from typing import Tuple, Any, Optional -from email.mime.multipart import MIMEMultipart +import random +import re +import string +import typing +from typing import Any, Optional, Tuple +import unicodedata def strip_accents(text): From f134d64349ce6a6013ac8787b55983959479f4f5 Mon Sep 17 00:00:00 2001 From: chrisbroderick Date: Thu, 8 Sep 2022 23:42:20 +0100 Subject: [PATCH 16/25] Add import sort checking and fixing to makefile --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 33c39c74..f34a0de5 100644 --- a/Makefile +++ b/Makefile @@ -64,12 +64,14 @@ test: .PHONY: format format: autopep8 --exit-code --global-config .flake8 helpdesk + isort --line-length=120 --src helpdesk . #: checkformat - checks formatting against configured format specifications for the project. .PHONY: checkformat checkformat: flake8 helpdesk --count --show-source --statistics --exit-zero --max-complexity=20 + isort --line-length=120 --src helpdesk . --check #: documentation - Build documentation (Sphinx, README, ...). From b3edba3fc5380fe877dff0de54b13cbaa6f2e7f7 Mon Sep 17 00:00:00 2001 From: Benbb96 Date: Sun, 9 Oct 2022 22:51:32 +0200 Subject: [PATCH 17/25] Fix #1054 --- helpdesk/email.py | 16 ++++++++++------ helpdesk/forms.py | 3 +++ helpdesk/tests/test_get_email.py | 27 ++++++++++++++++++++++++--- helpdesk/tests/utils.py | 8 +++----- helpdesk/validators.py | 4 +++- 5 files changed, 43 insertions(+), 15 deletions(-) diff --git a/helpdesk/email.py b/helpdesk/email.py index 85dd07d4..428ffa70 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -478,12 +478,16 @@ def create_object_from_email_message(message, ticket_id, payload, files, logger) logger.info("[%s-%s] %s" % (ticket.queue.slug, ticket.id, ticket.title,)) - attached = process_attachments(f, files) - for att_file in attached: - logger.info( - "Attachment '%s' (with size %s) successfully added to ticket from email.", - att_file[0], att_file[1].size - ) + try: + attached = process_attachments(f, files) + except ValidationError as e: + logger.error(str(e)) + else: + for att_file in attached: + logger.info( + "Attachment '%s' (with size %s) successfully added to ticket from email.", + att_file[0], att_file[1].size + ) context = safe_template_context(ticket) diff --git a/helpdesk/forms.py b/helpdesk/forms.py index 11ec89ed..8a6781d2 100644 --- a/helpdesk/forms.py +++ b/helpdesk/forms.py @@ -7,6 +7,7 @@ forms.py - Definitions of newforms-based forms for creating and maintaining tickets. """ + from datetime import datetime from django import forms from django.conf import settings @@ -33,6 +34,7 @@ from helpdesk.settings import ( CUSTOMFIELD_TIME_FORMAT, CUSTOMFIELD_TO_FIELD_DICT ) +from helpdesk.validators import validate_file_extension import logging @@ -243,6 +245,7 @@ class AbstractTicketForm(CustomFieldMixin, forms.Form): 'Only file types such as plain text (.txt), ' 'a document (.pdf, .docx, or .odt), ' 'or screenshot (.png or .jpg) may be uploaded.'), + validators=[validate_file_extension] ) class Media: diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index a3ae7aee..0c298fa1 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -3,6 +3,7 @@ from django.contrib.auth.hashers import make_password from django.contrib.auth.models import User +from django.core import mail from django.core.files.uploadedfile import SimpleUploadedFile from django.core.management import call_command from django.shortcuts import get_object_or_404 @@ -36,7 +37,7 @@ unused_port = "49151" class GetEmailCommonTests(TestCase): def setUp(self): - self.queue_public = Queue.objects.create() + self.queue_public = Queue.objects.create(title='Test', slug='test') self.logger = logging.getLogger('helpdesk') # tests correct syntax for command line option @@ -147,7 +148,7 @@ class GetEmailCommonTests(TestCase): ignore = IgnoreEmail(name="Test Ignore", email_address=from_meta[1], keep_in_mailbox=False) ignore.save() with self.assertRaises(DeleteIgnoredTicketException): - object_from_message(message.as_string(), self.queue_public, self.logger); + object_from_message(message.as_string(), self.queue_public, self.logger) def test_will_not_delete_ignored_email(self): """ @@ -158,7 +159,7 @@ class GetEmailCommonTests(TestCase): ignore = IgnoreEmail(name="Test Ignore", email_address=from_meta[1], keep_in_mailbox=True) ignore.save() with self.assertRaises(IgnoreTicketException): - object_from_message(message.as_string(), self.queue_public, self.logger); + object_from_message(message.as_string(), self.queue_public, self.logger) def test_utf8_filename_attachment(self): """ @@ -172,6 +173,26 @@ class GetEmailCommonTests(TestCase): # The extractor prepends a part identifier so compare the ending self.assertTrue(sent_file.name.endswith(filename), f"Filename extracted does not match: {sent_file.name}") + @override_settings(VALID_EXTENSIONS=['.png']) + def test_wrong_extension_attachment(self): + """ + Tests if an attachment with a wrong extension doesn't stop the email process + """ + message, _, _ = utils.generate_multipart_email(type_list=['plain', 'image']) + + self.assertEqual(len(mail.outbox), 0) + + with self.assertLogs(logger='helpdesk', level='ERROR') as cm: + object_from_message(message.as_string(), self.queue_public, self.logger) + + self.assertIn( + "ERROR:helpdesk:{'file': ['Unsupported file extension: .jpg']}", + cm.output + ) + + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(f'[test-1] {message.get("subject")} (Opened)', mail.outbox[0].subject) + class GetEmailParametricTemplate(object): """TestCase that checks basic email functionality across methods and socks configs.""" diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py index 00b7e6a5..88f0f990 100644 --- a/helpdesk/tests/utils.py +++ b/helpdesk/tests/utils.py @@ -161,7 +161,7 @@ def generate_image_mime_part(locale: str="en_US",imagename: str = None) -> Messa :param locale: change this to generate locale specific file name and attachment content :param filename: pass a file name if you want to specify a specific name otherwise a random name will be generated """ - part = MIMEImage(generate_random_image(image_format="JPEG")) + part = MIMEImage(generate_random_image(image_format="JPEG", array_dims=(200, 200))) part.set_payload(get_fake("text", locale=locale, min_length=1024)) encoders.encode_base64(part) if not imagename: @@ -206,7 +206,7 @@ def add_simple_email_headers(message: Message, locale: str="en_US", def generate_mime_part(locale: str="en_US", part_type: str="plain", use_short_email: bool=False - ) -> typing.Tuple[Message, typing.Tuple[str, str], typing.Tuple[str, str]]: + ) -> typing.Optional[Message]: """ Generates amime part of the sepecified type @@ -222,10 +222,8 @@ def generate_mime_part(locale: str="en_US", msg = MIMEText(body) elif "file" == part_type: msg = generate_file_mime_part(locale=locale) - msg = MIMEText(body) elif "image" == part_type: msg = generate_image_mime_part(locale=locale) - msg = MIMEText(body) else: raise Exception("Mime part not implemented: " + part_type) return msg @@ -243,7 +241,7 @@ def generate_multipart_email(locale: str="en_US", """ msg = MIMEMultipart() for part_type in type_list: - msg.append(generate_mime_part(locale=locale, part_type=part_type, use_short_email=use_short_email)) + msg.attach(generate_mime_part(locale=locale, part_type=part_type, use_short_email=use_short_email)) from_meta, to_meta = add_simple_email_headers(msg, locale=locale, use_short_email=use_short_email) return msg, from_meta, to_meta diff --git a/helpdesk/validators.py b/helpdesk/validators.py index 08086e1f..5fb80e39 100644 --- a/helpdesk/validators.py +++ b/helpdesk/validators.py @@ -4,6 +4,7 @@ from django.conf import settings +from django.utils.translation import gettext as _ # TODO: can we use the builtin Django validator instead? @@ -32,4 +33,5 @@ def validate_file_extension(value): # should always allow that? if not (ext.lower() == '' or ext.lower() == '.'): raise ValidationError( - 'Unsupported file extension: %s.' % ext.lower()) + _('Unsupported file extension: ') + ext.lower() + ) From 64788938b458d506755a8742a2ba374962c6da8c Mon Sep 17 00:00:00 2001 From: Benbb96 Date: Mon, 10 Oct 2022 21:57:56 +0200 Subject: [PATCH 18/25] Catch exception on an attachment individually in order to allow valid extension to be processed. --- helpdesk/lib.py | 23 ++++++++++++------ helpdesk/tests/test_get_email.py | 40 +++++++++++++++++++++++++++++++- helpdesk/tests/utils.py | 6 ++--- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/helpdesk/lib.py b/helpdesk/lib.py index 028d6353..f08aa23d 100644 --- a/helpdesk/lib.py +++ b/helpdesk/lib.py @@ -9,6 +9,7 @@ lib.py - Common functions (eg multipart e-mail) from datetime import date, datetime, time from django.conf import settings +from django.core.exceptions import ValidationError from django.utils.encoding import smart_str from helpdesk.settings import CUSTOMFIELD_DATE_FORMAT, CUSTOMFIELD_DATETIME_FORMAT, CUSTOMFIELD_TIME_FORMAT import logging @@ -132,6 +133,7 @@ def process_attachments(followup, attached_files): max_email_attachment_size = getattr( settings, 'HELPDESK_MAX_EMAIL_ATTACHMENT_SIZE', 512000) attachments = [] + errors = set() for attached in attached_files: @@ -148,14 +150,21 @@ def process_attachments(followup, attached_files): 'application/octet-stream', size=attached.size, ) - att.full_clean() - att.save() + try: + att.full_clean() + except ValidationError as e: + errors.add(e) + else: + att.save() - if attached.size < max_email_attachment_size: - # Only files smaller than 512kb (or as defined in - # settings.HELPDESK_MAX_EMAIL_ATTACHMENT_SIZE) are sent via - # email. - attachments.append([filename, att.file]) + if attached.size < max_email_attachment_size: + # Only files smaller than 512kb (or as defined in + # settings.HELPDESK_MAX_EMAIL_ATTACHMENT_SIZE) are sent via + # email. + attachments.append([filename, att.file]) + + if errors: + raise ValidationError(list(errors)) return attachments diff --git a/helpdesk/tests/test_get_email.py b/helpdesk/tests/test_get_email.py index 0c298fa1..0fe945ff 100644 --- a/helpdesk/tests/test_get_email.py +++ b/helpdesk/tests/test_get_email.py @@ -186,13 +186,51 @@ class GetEmailCommonTests(TestCase): object_from_message(message.as_string(), self.queue_public, self.logger) self.assertIn( - "ERROR:helpdesk:{'file': ['Unsupported file extension: .jpg']}", + "ERROR:helpdesk:['Unsupported file extension: .jpg']", cm.output ) self.assertEqual(len(mail.outbox), 1) self.assertEqual(f'[test-1] {message.get("subject")} (Opened)', mail.outbox[0].subject) + def test_multiple_attachments(self): + """ + Tests the saving of multiple attachments + """ + message, _, _ = utils.generate_multipart_email(type_list=['plain', 'file', 'image']) + + self.assertEqual(len(mail.outbox), 0) + + object_from_message(message.as_string(), self.queue_public, self.logger) + + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(f'[test-1] {message.get("subject")} (Opened)', mail.outbox[0].subject) + + ticket = Ticket.objects.get() + followup = ticket.followup_set.get() + self.assertEqual(2, followup.followupattachment_set.count()) + + @override_settings(VALID_EXTENSIONS=['.txt']) + def test_multiple_attachments_with_wrong_extension(self): + """ + Tests that a wrong extension won't stop from saving other valid attachment + """ + message, _, _ = utils.generate_multipart_email(type_list=['plain', 'image', 'file', 'image']) + + self.assertEqual(len(mail.outbox), 0) + + with self.assertLogs(logger='helpdesk', level='ERROR') as cm: + object_from_message(message.as_string(), self.queue_public, self.logger) + + self.assertIn( + "ERROR:helpdesk:['Unsupported file extension: .jpg']", + cm.output + ) + + ticket = Ticket.objects.get() + followup = ticket.followup_set.get() + self.assertEqual(1, followup.followupattachment_set.count()) + class GetEmailParametricTemplate(object): """TestCase that checks basic email functionality across methods and socks configs.""" diff --git a/helpdesk/tests/utils.py b/helpdesk/tests/utils.py index 88f0f990..f2822732 100644 --- a/helpdesk/tests/utils.py +++ b/helpdesk/tests/utils.py @@ -205,16 +205,14 @@ def add_simple_email_headers(message: Message, locale: str="en_US", def generate_mime_part(locale: str="en_US", part_type: str="plain", - use_short_email: bool=False ) -> typing.Optional[Message]: """ Generates amime part of the sepecified type :param locale: change this to generate locale specific strings :param text_type: options are plain, html, image (attachment), file (attachment) - :param use_short_email: produces a "To" or "From" that is only the email address if True """ - if "plain" == part_type: + if "plain" == part_type: body = get_fake("text", locale=locale, min_length=1024) msg = MIMEText(body) elif "html" == part_type: @@ -241,7 +239,7 @@ def generate_multipart_email(locale: str="en_US", """ msg = MIMEMultipart() for part_type in type_list: - msg.attach(generate_mime_part(locale=locale, part_type=part_type, use_short_email=use_short_email)) + msg.attach(generate_mime_part(locale=locale, part_type=part_type)) from_meta, to_meta = add_simple_email_headers(msg, locale=locale, use_short_email=use_short_email) return msg, from_meta, to_meta From c3aacbd813f30fa1de98e074eee745ab3c5e1ac9 Mon Sep 17 00:00:00 2001 From: Garret Wassermann Date: Mon, 7 Nov 2022 13:42:30 -0500 Subject: [PATCH 19/25] Remove link to heroku demo --- README.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/README.rst b/README.rst index 9cfbd9a5..fcbaae70 100644 --- a/README.rst +++ b/README.rst @@ -17,9 +17,6 @@ contributors reaching far beyond Jutda. Complete documentation is available in the docs/ directory, or online at http://django-helpdesk.readthedocs.org/. -You can see a demo installation at https://django-helpdesk-demo.herokuapp.com/, -or run a demo locally in just a couple steps! - Demo Quickstart --------------- From 2cca51085f4efd5b8aad7775b46fa8985bb645aa Mon Sep 17 00:00:00 2001 From: Rafael Reuber Date: Thu, 5 Jan 2023 22:30:57 +0000 Subject: [PATCH 20/25] Removes unused function query_to_dict --- helpdesk/query.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/helpdesk/query.py b/helpdesk/query.py index 9ebef3e9..9ae9e400 100644 --- a/helpdesk/query.py +++ b/helpdesk/query.py @@ -27,27 +27,6 @@ def query_from_base64(b64data): return query -def query_to_dict(results, descriptions): - """ - Replacement method for cursor.dictfetchall() as that method no longer - exists in psycopg2, and I'm guessing in other backends too. - - Converts the results of a raw SQL query into a list of dictionaries, suitable - for use in templates etc. - """ - - output = [] - for data in results: - row = {} - i = 0 - for column in descriptions: - row[column[0]] = data[i] - i += 1 - - output.append(row) - return output - - def get_search_filter_args(search): if search.startswith('queue:'): return Q(queue__title__icontains=search[len('queue:'):]) From 71ff0aa6d5f76e3581499d9d008e934ee5a360ca Mon Sep 17 00:00:00 2001 From: finnertysea <26181241+finnertysea@users.noreply.github.com> Date: Fri, 20 Jan 2023 10:53:35 -0500 Subject: [PATCH 21/25] Bug fix - detected due date change when no change was made --- helpdesk/views/staff.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index f31b89f6..bf274ad9 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -25,6 +25,8 @@ from django.urls import reverse, reverse_lazy from django.utils import timezone from django.utils.html import escape from django.utils.translation import gettext as _ +from django.utils.dateparse import parse_datetime +from django.utils.timezone import make_aware from django.views.decorators.csrf import requires_csrf_token from django.views.generic.edit import FormView, UpdateView from helpdesk import settings as helpdesk_settings @@ -536,12 +538,7 @@ def get_due_date_from_request_or_ticket( due_date = request.POST.get('due_date', None) or None if due_date is not None: - # based on Django code to parse dates: - # https://docs.djangoproject.com/en/2.0/_modules/django/utils/dateparse/ - match = DATE_RE.match(due_date) - if match: - kw = {k: int(v) for k, v in match.groupdict().items()} - due_date = date(**kw) + due_date = make_aware(parse_datetime(due_date)) else: due_date_year = int(request.POST.get('due_date_year', 0)) due_date_month = int(request.POST.get('due_date_month', 0)) From 527918022129859b837090850384c16cdb10969f Mon Sep 17 00:00:00 2001 From: finnertysea <26181241+finnertysea@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:36:18 -0500 Subject: [PATCH 22/25] Bug fix #1066 - modified import order to placate isort --- helpdesk/views/staff.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helpdesk/views/staff.py b/helpdesk/views/staff.py index bf274ad9..3602536d 100644 --- a/helpdesk/views/staff.py +++ b/helpdesk/views/staff.py @@ -23,10 +23,10 @@ from django.http import Http404, HttpResponse, HttpResponseRedirect, JsonRespons from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse, reverse_lazy from django.utils import timezone -from django.utils.html import escape -from django.utils.translation import gettext as _ from django.utils.dateparse import parse_datetime +from django.utils.html import escape from django.utils.timezone import make_aware +from django.utils.translation import gettext as _ from django.views.decorators.csrf import requires_csrf_token from django.views.generic.edit import FormView, UpdateView from helpdesk import settings as helpdesk_settings From 66a83a07169b92240b4922eba125782e41c11761 Mon Sep 17 00:00:00 2001 From: finnertysea <26181241+finnertysea@users.noreply.github.com> Date: Fri, 27 Jan 2023 12:33:33 -0800 Subject: [PATCH 23/25] #1069 - make regex matching username more permissive for rss_user to match rss_user_queue --- helpdesk/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helpdesk/urls.py b/helpdesk/urls.py index f0fcceab..1d001e18 100644 --- a/helpdesk/urls.py +++ b/helpdesk/urls.py @@ -151,7 +151,7 @@ urlpatterns += [ urlpatterns += [ re_path( - r"^rss/user/(?P[a-zA-Z0-9\_\.]+)/", + r"^rss/user/(?P[^/]+)/", helpdesk_staff_member_required(feeds.OpenTicketsByUser()), name="rss_user", ), From aad73a5d90421d59ab71189004925cfc7fbf2884 Mon Sep 17 00:00:00 2001 From: Christopher Broderick Date: Fri, 10 Mar 2023 22:06:14 +0000 Subject: [PATCH 24/25] Fix format errors --- helpdesk/email.py | 67 +++++++++++++++++++++++++---------------------- helpdesk/lib.py | 4 +-- 2 files changed, 37 insertions(+), 34 deletions(-) diff --git a/helpdesk/email.py b/helpdesk/email.py index 428ffa70..af8ee822 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -499,41 +499,44 @@ def create_object_from_email_message(message, ticket_id, payload, files, logger) logger.info( "Message seems to be auto-reply, not sending any emails back to the sender") else: - # send mail to appropriate people now depending on what objects - # were created and who was CC'd - # Add auto-reply headers because it's an auto-reply and we must - extra_headers = { - 'In-Reply-To': message_id, - "Auto-Submitted": "auto-replied", - "X-Auto-Response-Suppress": "All", - "Precedence": "auto_reply", - } - if new: - ticket.send( - {'submitter': ('newticket_submitter', context), - 'new_ticket_cc': ('newticket_cc', context), - 'ticket_cc': ('newticket_cc', context)}, - fail_silently=True, - extra_headers=extra_headers, - ) - else: - context.update(comment=f.comment) - ticket.send( - {'submitter': ('newticket_submitter', context), - 'assigned_to': ('updated_owner', context)}, - fail_silently=True, - extra_headers=extra_headers, - ) - if queue.enable_notifications_on_email_events: - ticket.send( - {'ticket_cc': ('updated_cc', context)}, - fail_silently=True, - extra_headers=extra_headers, - ) - + send_info_email(message_id, f, context, queue, new) return ticket +def send_info_email(message_id: str, f: FollowUp, ticket: Ticket, context: dict, queue: dict, new: bool): + # send mail to appropriate people now depending on what objects + # were created and who was CC'd + # Add auto-reply headers because it's an auto-reply and we must + extra_headers = { + 'In-Reply-To': message_id, + "Auto-Submitted": "auto-replied", + "X-Auto-Response-Suppress": "All", + "Precedence": "auto_reply", + } + if new: + ticket.send( + {'submitter': ('newticket_submitter', context), + 'new_ticket_cc': ('newticket_cc', context), + 'ticket_cc': ('newticket_cc', context)}, + fail_silently=True, + extra_headers=extra_headers, + ) + else: + context.update(comment=f.comment) + ticket.send( + {'submitter': ('newticket_submitter', context), + 'assigned_to': ('updated_owner', context)}, + fail_silently=True, + extra_headers=extra_headers, + ) + if queue.enable_notifications_on_email_events: + ticket.send( + {'ticket_cc': ('updated_cc', context)}, + fail_silently=True, + extra_headers=extra_headers, + ) + + def get_ticket_id_from_subject_slug( queue_slug: str, subject: str, diff --git a/helpdesk/lib.py b/helpdesk/lib.py index f08aa23d..d265963b 100644 --- a/helpdesk/lib.py +++ b/helpdesk/lib.py @@ -81,12 +81,12 @@ def text_is_spam(text, request): # This will return 'True' is the given text is deemed to be spam, or # False if it is not spam. If it cannot be checked for some reason, we # assume it isn't spam. - from django.contrib.sites.models import Site - from django.core.exceptions import ImproperlyConfigured try: from akismet import Akismet except ImportError: return False + from django.contrib.sites.models import Site + from django.core.exceptions import ImproperlyConfigured try: site = Site.objects.get_current() except ImproperlyConfigured: From 7f8e996668eea1c8d6c251c6a922141015b725eb Mon Sep 17 00:00:00 2001 From: Christopher Broderick Date: Fri, 10 Mar 2023 22:11:34 +0000 Subject: [PATCH 25/25] Fix missing param in call --- helpdesk/email.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helpdesk/email.py b/helpdesk/email.py index af8ee822..f27dc969 100644 --- a/helpdesk/email.py +++ b/helpdesk/email.py @@ -499,7 +499,7 @@ def create_object_from_email_message(message, ticket_id, payload, files, logger) logger.info( "Message seems to be auto-reply, not sending any emails back to the sender") else: - send_info_email(message_id, f, context, queue, new) + send_info_email(message_id, f, ticket, context, queue, new) return ticket