Implement mail tracker system

* Implement mail tracking

Signed-off-by: Victor "multun" Collod <victor.collod@prologin.org>

* Implement task merging

* Add a mail tracker title format pattern

* Autocomplete task names

* Fix comment display

* Track notification answers

* Add a socket timeout for the mail worker

A mail worker is a long running application. And sometimes, the IMAP server
just hangs for hours for no apparent reason. imaplib doesn't enable setting
a timeout, and setting it globally seems fine.

* Only validate the merge form when submitted

* Redirect to the new form when merging

* Prettier task edit UI

* Make task merging optional

* Test mail tracking

* Update documentation for mail tracking

* Update dependencies

* Add the TODO_COMMENT_CLASSES setting

* Fix dependencies install order

* Remove debug leftovers, improve documentation

* Fail on missing from_address

.travis.yml

@@ -9,9 +9,9 @@ addons:
   postgresql: "9.6"
 
 install:
-  - "pip3 install -e . --upgrade"
-  - "pip3 install git+https://github.com/pypa/pipenv.git"
+  - "pip3 install pipenv"
   - "pipenv install --dev"
+  - "pip3 install -e . --upgrade"
 
 language: python
 python:

Pipfile

@@ -11,6 +11,8 @@ django-extensions = "*"
 factory-boy = "*"
 titlecase = "*"
 bleach = "*"
+django-autocomplete-light = "*"
+html2text = "*"
 
 [dev-packages]
 mypy = "*"

Pipfile.lock

@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "12d4ee05e41a375e62fe7f3b8256abc7ffb047263e5d91291dced74be8b64192"
+            "sha256": "c6fb601fc8a197ca280960d831a5386313c93ebe19d932afa01034d5520f2f94"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -32,6 +32,13 @@
             "index": "pypi",
             "version": "==2.1.7"
         },
+        "django-autocomplete-light": {
+            "hashes": [
+                "sha256:996cc62519a6e2e9cd1c26e57ddc5f14541209a93e62e83d7b3df3ba65c1f458"
+            ],
+            "index": "pypi",
+            "version": "==3.3.2"
+        },
         "django-extensions": {
             "hashes": [
                 "sha256:109004f80b6f45ad1f56addaa59debca91d94aa0dc1cb19678b9364b4fe9b6f4",
@@ -70,6 +77,14 @@
             "index": "pypi",
             "version": "==3.7.7"
         },
+        "html2text": {
+            "hashes": [
+                "sha256:490db40fe5b2cd79c461cf56be4d39eb8ca68191ae41ba3ba79f6cb05b7dd662",
+                "sha256:627514fb30e7566b37be6900df26c2c78a030cc9e6211bda604d8181233bcdd4"
+            ],
+            "index": "pypi",
+            "version": "==2018.1.9"
+        },
         "mccabe": {
             "hashes": [
                 "sha256:ab8a6258860da4b6677da4bd2fe5dc2c659cff31b3ee4f7f5d64e79735b80d42",

README.md

@@ -23,6 +23,7 @@ assignment application for Django, designed to be dropped into an existing site
 * jQuery (full version, not "slim", for drag/drop prioritization)
 * Bootstrap (to work with provided templates, though you can override them)
 * bleach (`pip install bleach`)
+* django-autocomplete-light (optional, required for task merging)
 
 ## Overview
 
@@ -53,7 +54,7 @@ If using your own site, be sure you have jQuery and Bootstrap wired up and worki
 django-todo pages that require it will insert additional CSS/JavaScript into page heads,
 so your project's base templates must include:
 
-```
+```jinja
 {% block extrahead %}{% endblock extrahead %}
 {% block extra_js %}{% endblock extra_js %}
 ```
@@ -107,7 +108,7 @@ If you wish to use the public ticket-filing system, first create the list into w
 
 Optional configuration options:
 
-```
+```python
 # Restrict access to ALL todo lists/views to `is_staff` users.
 # If False or unset, all users can see all views (but more granular permissions are still enforced
 # within views, such as requiring staff for adding and deleting lists).
@@ -126,6 +127,9 @@ TODO_DEFAULT_LIST_SLUG = 'tickets'
 # Defaults to "/"
 TODO_PUBLIC_SUBMIT_REDIRECT = 'dashboard'
 
+# additionnal classes the comment body should hold
+# adding "text-monospace" makes comment monospace
+TODO_COMMENT_CLASSES = []
 ```
 
 The current django-todo version number is available from the [todo package](https://github.com/shacker/django-todo/blob/master/todo/__init__.py):
@@ -133,6 +137,95 @@ The current django-todo version number is available from the [todo package](http
     python -c "import todo; print(todo.__version__)"
 
 
+## Mail tracking
+
+What if you could turn django-todo into a shared mailbox?
+Django-todo includes an optional feature that allows emails sent to a
+dedicated mailbox to be pushed into todo as new tasks, and responses to
+be added as comments on that original tasks.
+
+This allows support teams to work with a fully unified email + bug
+tracking system to avoid confusion over who's seen or responded to what.
+
+To enable the feature, you need to:
+
+ - define an email backend for outgoing emails
+ - define an email backend for incoming emails
+ - start a worker, which will wait for new emails
+
+```python
+from todo.mail.producers import imap_producer
+from todo.mail.consumers import tracker_consumer
+from todo.mail.delivery import smtp_backend, console_backend
+
+# email notifications configuration
+# each task list can get its own delivery method
+TODO_MAIL_BACKENDS = {
+    # mail-queue is the name of the task list, not the worker name
+    "mail-queue": smtp_backend(
+        host="smtp.example.com",
+        port=465,
+        use_ssl=True,
+        username="test@example.com",
+        password="foobar",
+        # used as the From field when sending notifications.
+        # a username might be prepended later on
+        from_address="test@example.com",
+        # additionnal headers
+        headers={}
+    ),
+}
+
+# incoming mail worker configuration
+TODO_MAIL_TRACKERS = {
+    # configuration for worker "test_tracker"
+    "test_tracker": {
+        "producer": imap_producer(
+            host="imap.example.com",
+            username="text@example.com",
+            password="foobar",
+            # process_all=False, # by default, only unseen emails are processed
+            # preserve=False, # delete emails if False
+            # nap_duration=1, # duration of the pause between polling rounds
+            # input_folder="INBOX", # where to read emails from
+        ),
+        "consumer": tracker_consumer(
+            group="Mail Queuers",
+            task_list_slug="mail-queue",
+            priority=1,
+            task_title_format="[TEST_MAIL] {subject}",
+        )
+    }
+}
+```
+
+A mail worker can be started this way:
+
+```sh
+./manage.py mail_worker test_tracker
+```
+
+If you want to log mail events, make sure to properly configure django logging:
+
+```python
+LOGGING = {
+    'version': 1,
+    'disable_existing_loggers': False,
+    'handlers': {
+        'console': {
+            'class': 'logging.StreamHandler',
+        },
+    },
+    'loggers': {
+        '': {
+            'handlers': ['console'],
+            'level': 'DEBUG',
+            'propagate': True,
+        },
+    },
+}
+```
+
 ## Upgrade Notes
 
 django-todo 2.0 was rebuilt almost from the ground up, and included some radical changes, including model name changes. As a result, it is *not compatible* with data from django-todo 1.x. If you would like to upgrade an existing installation, try this:
@@ -229,5 +322,3 @@ ALL groups, not just the groups they "belong" to)
 **0.9.1** - Removed context_processors.py - leftover turdlet
 
 **0.9** - First release
-
-

test_settings.py

@@ -29,6 +29,8 @@ INSTALLED_APPS = (
     "django.contrib.sites",
     "django.contrib.staticfiles",
     "todo",
+    "dal",
+    "dal_select2",
 )
 
 ROOT_URLCONF = "base_urls"
@@ -61,3 +63,30 @@ TEMPLATES = [
         },
     }
 ]
+
+LOGGING = {
+    'version': 1,
+    'disable_existing_loggers': False,
+    'handlers': {
+        'console': {
+            'class': 'logging.StreamHandler',
+        },
+    },
+    'loggers': {
+        '': {
+            'handlers': ['console'],
+            'level': 'DEBUG',
+            'propagate': True,
+        },
+        'django': {
+            'handlers': ['console'],
+            'level': 'DEBUG',
+            'propagate': True,
+        },
+        'django.request': {
+            'handlers': ['console'],
+            'level': 'DEBUG',
+            'propagate': True,
+        },
+    },
+}

todo/__init__.py

@@ -8,3 +8,5 @@ __email__ = 'shacker@birdhouse.org'
 
 __url__ = 'https://github.com/shacker/django-todo'
 __license__ = 'BSD License'
+
+from . import check

todo/check.py

@@ -0,0 +1,19 @@
+from django.core.checks import Error, register
+
+# the sole purpose of this warning is to prevent people who have
+# django-autocomplete-light installed but not configured to start the app
+@register()
+def dal_check(app_configs, **kwargs):
+    from django.conf import settings
+    from todo.features import HAS_AUTOCOMPLETE
+
+    if not HAS_AUTOCOMPLETE:
+        return
+
+    errors = []
+    missing_apps = {'dal', 'dal_select2'} - set(settings.INSTALLED_APPS)
+    for missing_app in missing_apps:
+        errors.append(
+            Error('{} needs to be in INSTALLED_APPS'.format(missing_app))
+        )
+    return errors

todo/features.py

@@ -0,0 +1,11 @@
+HAS_AUTOCOMPLETE = True
+try:
+    import dal
+except ImportError:
+    HAS_AUTOCOMPLETE = False
+
+HAS_TASK_MERGE = False
+if HAS_AUTOCOMPLETE:
+    import dal.autocomplete
+    if getattr(dal.autocomplete, 'Select2QuerySetView', None) is not None:
+        HAS_TASK_MERGE = True

todo/mail/consumers/__init__.py

@@ -0,0 +1,9 @@
+def tracker_consumer(**kwargs):
+    def tracker_factory(producer):
+        # the import needs to be delayed until call to enable
+        # using the wrapper in the django settings
+        from .tracker import tracker_consumer
+
+        return tracker_consumer(producer, **kwargs)
+
+    return tracker_factory

todo/mail/consumers/tracker.py

@@ -0,0 +1,150 @@
+import re
+import logging
+
+from email.charset import Charset as EMailCharset
+from django.db import transaction
+from django.db.models import Count
+from html2text import html2text
+from todo.models import Comment, Task, TaskList
+
+logger = logging.getLogger(__name__)
+
+
+def part_decode(message):
+    charset = ("ascii", "ignore")
+    email_charset = message.get_content_charset()
+    if email_charset:
+        charset = (EMailCharset(email_charset).input_charset,)
+
+    body = message.get_payload(decode=True)
+    return body.decode(*charset)
+
+
+def message_find_mime(message, mime_type):
+    for submessage in message.walk():
+        if submessage.get_content_type() == mime_type:
+            return submessage
+    return None
+
+
+def message_text(message):
+    text_part = message_find_mime(message, "text/plain")
+    if text_part is not None:
+        return part_decode(text_part)
+
+    html_part = message_find_mime(message, "text/html")
+    if html_part is not None:
+        return html2text(part_decode(html_part))
+
+    # TODO: find something smart to do when no text if found
+    return ""
+
+
+def format_task_title(format_string, message):
+    return format_string.format(
+        subject=message["subject"],
+        author=message["from"],
+    )
+
+
+DJANGO_TODO_THREAD = re.compile(r'<thread-(\d+)@django-todo>')
+
+
+def parse_references(task_list, references):
+    related_messages = []
+    answer_thread = None
+    for related_message in references.split():
+        logger.info("checking reference: %r", related_message)
+        match = re.match(DJANGO_TODO_THREAD, related_message)
+        if match is None:
+            related_messages.append(related_message)
+            continue
+
+        thread_id = int(match.group(1))
+        new_answer_thread = Task.objects.filter(
+            task_list=task_list,
+            pk=thread_id
+        ).first()
+        if new_answer_thread is not None:
+            answer_thread = new_answer_thread
+
+    if answer_thread is None:
+        logger.info("no answer thread found in references")
+    else:
+        logger.info("found an answer thread: %d", answer_thread)
+    return related_messages, answer_thread
+
+
+def insert_message(task_list, message, priority, task_title_format):
+    if "message-id" not in message:
+        logger.warning("missing message id, ignoring message")
+        return
+
+    if "from" not in message:
+        logger.warning('missing "From" header, ignoring message')
+        return
+
+    if "subject" not in message:
+        logger.warning('missing "Subject" header, ignoring message')
+        return
+
+    logger.info(
+        "received message:\t"
+        f"[Subject: {message['subject']}]\t"
+        f"[Message-ID: {message['message-id']}]\t"
+        f"[References: {message['references']}]\t"
+        f"[To: {message['to']}]\t"
+        f"[From: {message['from']}]"
+    )
+
+    message_id = message["message-id"]
+    message_from = message["from"]
+    text = message_text(message)
+
+    related_messages, answer_thread = \
+        parse_references(task_list, message.get("references", ""))
+
+    # find the most relevant task to add a comment on.
+    # among tasks in the selected task list, find the task having the
+    # most email comments the current message references
+    best_task = (
+        Task.objects.filter(
+            task_list=task_list, comment__email_message_id__in=related_messages
+        )
+        .annotate(num_comments=Count("comment"))
+        .order_by("-num_comments")
+        .only("id")
+        .first()
+    )
+
+    # if no related comment is found but a thread message-id
+    # (generated by django-todo) could be found, use it
+    if best_task is None and answer_thread is not None:
+        best_task = answer_thread
+
+    with transaction.atomic():
+        if best_task is None:
+            best_task = Task.objects.create(
+                priority=priority,
+                title=format_task_title(task_title_format, message),
+                task_list=task_list
+            )
+        logger.info("using task: %r", best_task)
+
+        comment, comment_created = Comment.objects.get_or_create(
+            task=best_task,
+            email_message_id=message_id,
+            defaults={"email_from": message_from, "body": text},
+        )
+        logger.info("created comment: %r", comment)
+
+
+def tracker_consumer(producer, group=None, task_list_slug=None,
+                     priority=1, task_title_format="[MAIL] {subject}"):
+    task_list = TaskList.objects.get(group__name=group, slug=task_list_slug)
+    for message in producer:
+        try:
+            insert_message(task_list, message, priority, task_title_format)
+        except Exception:
+            # ignore exceptions during insertion, in order to avoid
+            logger.exception("got exception while inserting message")

todo/mail/delivery.py

@@ -0,0 +1,25 @@
+import importlib
+
+def _declare_backend(backend_path):
+    backend_path = backend_path.split('.')
+    backend_module_name = '.'.join(backend_path[:-1])
+    class_name = backend_path[-1]
+
+    def backend(*args, headers={}, from_address=None, **kwargs):
+        def _backend():
+            backend_module = importlib.import_module(backend_module_name)
+            backend = getattr(backend_module, class_name)
+            return backend(*args, **kwargs)
+
+        if from_address is None:
+            raise ValueError("missing from_address")
+
+        _backend.from_address = from_address
+        _backend.headers = headers
+        return _backend
+    return backend
+
+
+smtp_backend = _declare_backend('django.core.mail.backends.smtp.EmailBackend')
+console_backend = _declare_backend('django.core.mail.backends.console.EmailBackend')
+locmem_backend = _declare_backend('django.core.mail.backends.locmem.EmailBackend')

todo/mail/producers/__init__.py

@@ -0,0 +1,9 @@
+def imap_producer(**kwargs):
+    def imap_producer_factory():
+        # the import needs to be delayed until call to enable
+        # using the wrapper in the django settings
+        from .imap import imap_producer
+
+        return imap_producer(**kwargs)
+
+    return imap_producer_factory

todo/mail/producers/imap.py

@@ -0,0 +1,100 @@
+import email
+import email.parser
+import imaplib
+import logging
+import time
+
+from email.policy import default
+from contextlib import contextmanager
+
+logger = logging.getLogger(__name__)
+
+
+def imap_check(command_tuple):
+    status, ids = command_tuple
+    assert status == "OK", ids
+
+
+@contextmanager
+def imap_connect(host, port, username, password):
+    conn = imaplib.IMAP4_SSL(host=host, port=port)
+    conn.login(username, password)
+    imap_check(conn.list())
+    try:
+        yield conn
+    finally:
+        conn.close()
+
+
+def parse_message(message):
+    for response_part in message:
+        if not isinstance(response_part, tuple):
+            continue
+
+        message_metadata, message_content = response_part
+        email_parser = email.parser.BytesFeedParser(policy=default)
+        email_parser.feed(message_content)
+        return email_parser.close()
+
+
+def search_message(conn, *filters):
+    status, message_ids = conn.search(None, *filters)
+    for message_id in message_ids[0].split():
+        status, message = conn.fetch(message_id, "(RFC822)")
+        yield message_id, parse_message(message)
+
+
+def imap_producer(
+    process_all=False,
+    preserve=False,
+    host=None,
+    port=993,
+    username=None,
+    password=None,
+    nap_duration=1,
+    input_folder="INBOX",
+):
+    logger.debug("starting IMAP worker")
+    imap_filter = "(ALL)" if process_all else "(UNSEEN)"
+
+    def process_batch():
+        logger.debug("starting to process batch")
+        # reconnect each time to avoid repeated failures due to a lost connection
+        with imap_connect(host, port, username, password) as conn:
+            # select the requested folder
+            imap_check(conn.select(input_folder, readonly=False))
+
+            try:
+                for message_uid, message in search_message(conn, imap_filter):
+                    logger.info(f"received message {message_uid}")
+                    try:
+                        yield message
+                    except Exception:
+                        logger.exception(
+                            f"something went wrong while processing {message_uid}"
+                        )
+                        raise
+
+                    if not preserve:
+                        # tag the message for deletion
+                        conn.uid("STORE", message_uid, "+FLAGS", "(\\Deleted)")
+                else:
+                    logger.debug("did not receive any message")
+            finally:
+                if not preserve:
+                    # flush deleted messages
+                    conn.expunge()
+
+    while True:
+        try:
+            yield from process_batch()
+        except (GeneratorExit, KeyboardInterrupt):
+            # the generator was closed, due to the consumer
+            # breaking out of the loop, or an exception occuring
+            raise
+        except Exception:
+            logger.exception("mail fetching went wrong, retrying")
+
+        # sleep to avoid using too much resources
+        # TODO: get notified when a new message arrives
+        time.sleep(nap_duration)

todo/management/commands/mail_worker.py

@@ -0,0 +1,44 @@
+import logging
+import socket
+import sys
+
+from django.core.management.base import BaseCommand
+from django.conf import settings
+
+logger = logging.getLogger(__name__)
+
+
+DEFAULT_IMAP_TIMEOUT = 20
+
+
+class Command(BaseCommand):
+    help = "Starts a mail worker"
+
+    def add_arguments(self, parser):
+        parser.add_argument("--imap_timeout", type=int, default=30)
+        parser.add_argument("worker_name")
+
+    def handle(self, *args, **options):
+        if not hasattr(settings, "TODO_MAIL_TRACKERS"):
+            logger.error("missing TODO_MAIL_TRACKERS setting")
+            sys.exit(1)
+
+        worker_name = options["worker_name"]
+        tracker = settings.TODO_MAIL_TRACKERS.get(worker_name, None)
+        if tracker is None:
+            logger.error(
+                "couldn't find configuration for %r in TODO_MAIL_TRACKERS",
+                worker_name
+            )
+            sys.exit(1)
+
+        # set the default socket timeout (imaplib doesn't enable configuring it)
+        timeout = options["imap_timeout"]
+        if timeout:
+            socket.setdefaulttimeout(timeout)
+
+        # run the mail polling loop
+        producer = tracker["producer"]
+        consumer = tracker["consumer"]
+
+        consumer(producer())

todo/migrations/0008_mail_tracker.py

@@ -0,0 +1,46 @@
+# Generated by Django 2.1.4 on 2018-12-21 14:01
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [("todo", "0007_auto_update_created_date")]
+
+    operations = [
+        migrations.AddField(
+            model_name="comment",
+            name="email_from",
+            field=models.CharField(blank=True, max_length=320, null=True),
+        ),
+        migrations.AddField(
+            model_name="comment",
+            name="email_message_id",
+            field=models.TextField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name="comment",
+            name="author",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                to=settings.AUTH_USER_MODEL,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="task",
+            name="created_by",
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="todo_created_by",
+                to=settings.AUTH_USER_MODEL,
+            ),
+        ),
+        migrations.AlterUniqueTogether(
+            name="comment", unique_together={("task", "email_message_id")}
+        ),
+    ]

todo/models.py

@@ -1,13 +1,50 @@
 from __future__ import unicode_literals
 import datetime
+import textwrap
 
 from django.conf import settings
 from django.contrib.auth.models import Group
-from django.db import models
+from django.db import models, DEFAULT_DB_ALIAS
+from django.db.transaction import Atomic, get_connection
 from django.urls import reverse
 from django.utils import timezone
 
 
+class LockedAtomicTransaction(Atomic):
+    """
+    modified from https://stackoverflow.com/a/41831049
+    this is needed for safely merging
+
+    Does a atomic transaction, but also locks the entire table for any transactions, for the duration of this
+    transaction. Although this is the only way to avoid concurrency issues in certain situations, it should be used with
+    caution, since it has impacts on performance, for obvious reasons...
+    """
+
+    def __init__(self, *models, using=None, savepoint=None):
+        if using is None:
+            using = DEFAULT_DB_ALIAS
+        super().__init__(using, savepoint)
+        self.models = models
+
+    def __enter__(self):
+        super(LockedAtomicTransaction, self).__enter__()
+
+        # Make sure not to lock, when sqlite is used, or you'll run into problems while running tests!!!
+        if settings.DATABASES[self.using]["ENGINE"] != "django.db.backends.sqlite3":
+            cursor = None
+            try:
+                cursor = get_connection(self.using).cursor()
+                for model in self.models:
+                    cursor.execute(
+                        "LOCK TABLE {table_name}".format(
+                            table_name=model._meta.db_table
+                        )
+                    )
+            finally:
+                if cursor and not cursor.closed:
+                    cursor.close()
+
+
 class TaskList(models.Model):
     name = models.CharField(max_length=60)
     slug = models.SlugField(default="")
@@ -32,7 +69,10 @@ class Task(models.Model):
     completed = models.BooleanField(default=False)
     completed_date = models.DateField(blank=True, null=True)
     created_by = models.ForeignKey(
-        settings.AUTH_USER_MODEL, related_name="todo_created_by", on_delete=models.CASCADE
+        settings.AUTH_USER_MODEL,
+        null=True,
+        related_name="todo_created_by",
+        on_delete=models.CASCADE,
     )
     assigned_to = models.ForeignKey(
         settings.AUTH_USER_MODEL,
@@ -63,6 +103,17 @@ class Task(models.Model):
             self.completed_date = datetime.datetime.now()
         super(Task, self).save()
 
+    def merge_into(self, merge_target):
+        if merge_target.pk == self.pk:
+            raise ValueError("can't merge a task with self")
+
+        # lock the comments to avoid concurrent additions of comments after the
+        # update request. these comments would be irremediably lost because of
+        # the cascade clause
+        with LockedAtomicTransaction(Comment):
+            Comment.objects.filter(task=self).update(task=merge_target)
+            self.delete()
+
     class Meta:
         ordering = ["priority"]
 
@@ -73,14 +124,35 @@ class Comment(models.Model):
     a comment and change task details at the same time. Rolling our own since it's easy.
     """
 
-    author = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
+    author = models.ForeignKey(
+        settings.AUTH_USER_MODEL, on_delete=models.CASCADE, blank=True, null=True
+    )
     task = models.ForeignKey(Task, on_delete=models.CASCADE)
     date = models.DateTimeField(default=datetime.datetime.now)
+    email_from = models.CharField(max_length=320, blank=True, null=True)
+    email_message_id = models.TextField(blank=True, null=True)
+
     body = models.TextField(blank=True)
 
+    class Meta:
+        # an email should only appear once per task
+        unique_together = ("task", "email_message_id")
+
+    @property
+    def author_text(self):
+        if self.author is not None:
+            return str(self.author)
+
+        assert self.email_message_id is not None
+        return str(self.email_from)
+
+    @property
     def snippet(self):
+        body_snippet = textwrap.shorten(self.body, width=35, placeholder="...")
         # Define here rather than in __str__ so we can use it in the admin list_display
-        return "{author} - {snippet}...".format(author=self.author, snippet=self.body[:35])
+        return "{author} - {snippet}...".format(
+            author=self.author_text, snippet=body_snippet
+        )
 
     def __str__(self):
-        return self.snippet()
+        return self.snippet

todo/templates/todo/include/task_edit.html

@@ -2,9 +2,7 @@
 
 <form action="" name="add_task" method="post">
   {% csrf_token %}
-
-
-  <div id="AddEditTask" class="collapse mt-3">
+  <div class="mt-3">
     <div class="form-group">
       <label for="id_title" name="title">Task</label>
       <input type="text" class="form-control" id="id_title" name="title" required placeholder="Task title"
@@ -33,13 +31,16 @@
     </div>
 
     <div class="form-group">
-      <label for="id_notify">Notify</label>
-      <input type="checkbox" checked="checked" class="form-control" id="id_notify" name="notify" aria-describedby="inputNotifyHelp"
-        value="{{ form.notify.text }}">
+      <div class="form-check">
+        <input class="form-check-input" type="checkbox" aria-describedby="inputNotifyHelp" checked="checked" id="id_notify">
+        <label class="form-check-label" for="id_notify">
+          Notify
+        </label>
         <small id="inputNotifyHelp" class="form-text text-muted">
           Email notifications will only be sent if task is assigned to someone other than yourself.
         </small>
       </div>
+    </div>
 
     <input type="hidden" name="priority"
       value="{% if form.priority.value %}{{ form.priority.value }}{% else %}999{% endif %}" id="id_priority">

todo/templates/todo/list_detail.html

@@ -10,7 +10,9 @@
       data-toggle="collapse" data-target="#AddEditTask">Add Task</button>
 
     {# Task edit / new task form #}
+    <div id="AddEditTask" class="collapse">
       {% include 'todo/include/task_edit.html' %}
+    </div>
     <hr />
   {% endif %}
 

todo/templates/todo/task_detail.html

@@ -2,29 +2,46 @@
 
 {% block title %}Task:{{ task.title }}{% endblock %}
 
+{% block extrahead %}
+<style>
+.select2 {
+    width: 100% !important;
+}
+
+.select2-container {
+    min-width: 0 !important;
+}
+</style>
+{{ form.media }}
+{{ merge_form.media }}
+{% endblock %}
+
+
+
 {% block content %}
-  <div class="row">
-    <div class="col-sm-8">
+  <div class="card-deck">
+    <div class="card col-sm-8">
+      <div class="card-body">
         <h3 class="card-title">{{ task.title }}</h3>
         {% if task.note %}
-        <p class="card-text">{{ task.note|safe|urlize|linebreaks }}</p>
+        <div class="card-text">{{ task.note|safe|urlize|linebreaks }}</div>
         {% endif %}
       </div>
+    </div>
 
-    <div class="col-sm-4">
-      <div class="mb-2">
-
+    <div class="card col-sm-4 p-0">
+      <ul class="list-group list-group-flush">
+        <li class="list-group-item">
           <button
             class="btn btn-sm btn-primary"
             id="EditTaskButton"
             type="button"
             data-toggle="collapse"
-          data-target="#AddEditTask"
-        >
+            data-target="#TaskEdit">
             Edit Task
           </button>
 
-        <form method="post" action="{% url "todo:task_toggle_done" task.id %}" role="form" style="display:inline;">
+          <form method="post" action="{% url "todo:task_toggle_done" task.id %}" role="form" class="d-inline">
             {% csrf_token %}
             <div style="display:inline;">
               <button class="btn btn-info btn-sm" type="submit" name="toggle_done">
@@ -33,7 +50,7 @@
             </div>
           </form>
 
-        <form method="post" action="{% url "todo:delete_task" task.id %}" role="form" style="display:inline;">
+          <form method="post" action="{% url "todo:delete_task" task.id %}" role="form" class="d-inline">
             {% csrf_token %}
             <div style="display:inline;">
               <button class="btn btn-danger btn-sm" type="submit" name="submit_delete">
@@ -41,11 +58,7 @@
               </button>
             </div>
           </form>
-
-      </div>
-
-      <ul class="list-group">
-
+        </li>
         <li class="list-group-item">
           <strong>Assigned to:</strong>
           {% if task.assigned_to %} {{ task.assigned_to.get_full_name }} {% else %} Anyone {% endif %}
@@ -77,11 +90,32 @@
     </div>
   </div>
 
-  <div id="TaskEdit">
+  <div id="TaskEdit" class="collapse">
     {# Task edit / new task form #}
     {% include 'todo/include/task_edit.html' %}
+    {% if merge_form is not None %}
+    <form action="" method="post">
+      <div class="card border-danger">
+      <div class="card-header">Merge task</div>
+      <div class="card-body">
+        <div class="">
+          <p>Merging is a destructive operation. This task will not exist anymore, and comments will be moved to the target task.</p>
+        {% csrf_token %}
+        {% for field in merge_form.visible_fields %}
+        <p>
+          {{ field.errors }}
+          {{ field }}
+        </p>
+        {% endfor %}
+        <input class="d-inline btn btn-sm btn-outline-danger" type="submit" name="merge_task_into" value="Merge">
+      </div>
+      </div>
+      </div>
+    </form>
+    {% endif %}
   </div>
 
+  <div class="mt-3">
     <h5>Add comment</h5>
     <form action="" method="post">
       {% csrf_token %}
@@ -90,22 +124,31 @@
       </div>
       <input class="btn btn-sm btn-primary" type="submit" name="add_comment" value="Add Comment">
     </form>
+  </div>
 
   <div class="task_comments mt-4">
     {% if comment_list %}
       <h5>Comments on this task</h5>
       {% for comment in comment_list %}
-        <p>
-          <strong>{{ comment.author.first_name }}
-            {{ comment.author.last_name }},
+      <div class="mb-3 card">
+        <div class="card-header">
+          <div class="float-left">
+            {% if comment.email_message_id %}
+            <span class="badge badge-warning">email</span>
+            {% endif %}
+            {{ comment.author_text }}
+          </div>
+          <span class="float-right d-inline-block text-muted">
             {{ comment.date|date:"F d Y P" }}
-          </strong>
-        </p>
+          </span>
+        </div>
+        <div class="{{ comment_classes | join:" " }} card-body">
           {{ comment.body|safe|urlize|linebreaks }}
+        </div>
+      </div>
       {% endfor %}
     {% else %}
         <h5>No comments (yet).</h5>
     {% endif %}
   </div>
-
 {% endblock %}

todo/tests/conftest.py

@@ -28,3 +28,9 @@ def todo_setup(django_user_model):
     Task.objects.create(created_by=u2, title="Task 1", task_list=tlist2, priority=1)
     Task.objects.create(created_by=u2, title="Task 2", task_list=tlist2, priority=2, completed=True)
     Task.objects.create(created_by=u2, title="Task 3", task_list=tlist2, priority=3)
+
+
+@pytest.fixture()
+# Set up an in-memory mail server to receive test emails
+def email_backend_setup(settings):
+    settings.EMAIL_BACKEND = "django.core.mail.backends.locmem.EmailBackend"

todo/tests/test_tracker.py

@@ -0,0 +1,67 @@
+import pytest
+
+from django.core import mail
+
+from todo.models import Task, Comment
+from todo.mail.consumers import tracker_consumer
+from email.message import EmailMessage
+
+
+def consumer(*args, title_format="[TEST] {subject}", **kwargs):
+    return tracker_consumer(
+        group="Workgroup One",
+        task_list_slug="zip",
+        priority=1,
+        task_title_format=title_format,
+    )(*args, **kwargs)
+
+
+def make_message(subject, content):
+    msg = EmailMessage()
+    msg.set_content(content)
+    msg['Subject'] = subject
+    return msg
+
+
+def test_tracker_task_creation(todo_setup, django_user_model):
+    msg = make_message("test1 subject", "test1 content")
+    msg['From'] = 'test1@example.com'
+    msg['Message-ID'] = '<a@example.com>'
+
+    # test task creation
+    task_count = Task.objects.count()
+    consumer([msg])
+
+    assert task_count + 1 == Task.objects.count(), "task wasn't created"
+    task = Task.objects.filter(title="[TEST] test1 subject").first()
+    assert task is not None, "task was created with the wrong name"
+
+    # test thread answers
+    msg = make_message("test2 subject", "test2 content")
+    msg['From'] = 'test1@example.com'
+    msg['Message-ID'] = '<b@example.com>'
+    msg['References'] = '<nope@example.com> <a@example.com>'
+
+    task_count = Task.objects.count()
+    consumer([msg])
+    assert task_count == Task.objects.count(), "comment created another task"
+    Comment.objects.get(
+        task=task,
+        body__contains="test2 content",
+        email_message_id='<b@example.com>'
+    )
+
+    # test notification answer
+    msg = make_message("test3 subject", "test3 content")
+    msg['From'] = 'test1@example.com'
+    msg['Message-ID'] = '<c@example.com>'
+    msg['References'] = '<thread-{}@django-todo> <unknown@example.com>'.format(task.pk)
+
+    task_count = Task.objects.count()
+    consumer([msg])
+    assert task_count == Task.objects.count(), "comment created another task"
+    Comment.objects.get(
+        task=task,
+        body__contains="test3 content",
+        email_message_id='<c@example.com>'
+    )

todo/tests/test_utils.py

@@ -6,12 +6,6 @@ from todo.models import Task, Comment
 from todo.utils import send_notify_mail, send_email_to_thread_participants
 
 
-@pytest.fixture()
-# Set up an in-memory mail server to receive test emails
-def email_backend_setup(settings):
-    settings.EMAIL_BACKEND = "django.core.mail.backends.locmem.EmailBackend"
-
-
 def test_send_notify_mail_not_me(todo_setup, django_user_model, email_backend_setup):
     """Assign a task to someone else, mail should be sent.
     TODO: Future tests could check for email contents.

todo/urls.py

@@ -1,11 +1,12 @@
 from django.urls import path
 
 from todo import views
-
+from todo.features import HAS_TASK_MERGE
 app_name = 'todo'
 
-urlpatterns = [
+from django.conf import settings
 
+urlpatterns = [
     path(
         '',
         views.list_lists,
@@ -55,7 +56,19 @@ urlpatterns = [
         'task/<int:task_id>/',
         views.task_detail,
         name='task_detail'),
+]
 
+if HAS_TASK_MERGE:
+    # ensure autocomplete is optional
+    from todo.views.task_autocomplete import TaskAutocomplete
+    urlpatterns.append(
+        path(
+            'task/<int:task_id>/autocomplete/',
+            TaskAutocomplete.as_view(),
+            name='task_autocomplete')
+    )
+
+urlpatterns.extend([
     path(
         'toggle_done/<int:task_id>/',
         views.toggle_done,
@@ -70,4 +83,4 @@ urlpatterns = [
         'search/',
         views.search,
         name="search"),
-]
+])

todo/utils.py

@@ -1,8 +1,12 @@
+import email.utils
+import functools
+import time
 from django.conf import settings
 from django.contrib.sites.models import Site
+from django.core import mail
+from django.core.exceptions import PermissionDenied
 from django.core.mail import send_mail
 from django.template.loader import render_to_string
-
 from todo.models import Comment, Task
 
 
@@ -19,45 +23,134 @@ def staff_check(user):
         return True
 
 
-def send_notify_mail(new_task):
-    # Send email to assignee if task is assigned to someone other than submittor.
-    # Unassigned tasks should not try to notify.
+def user_can_read_task(task, user):
+    return task.task_list.group in user.groups.all() or user.is_staff
+
+
+def todo_get_backend(task):
+    '''returns a mail backend for some task'''
+    mail_backends = getattr(settings, "TODO_MAIL_BACKENDS", None)
+    if mail_backends is None:
+        return None
+
+    task_backend = mail_backends[task.task_list.slug]
+    if task_backend is None:
+        return None
+
+    return task_backend
+
+
+def todo_get_mailer(user, task):
+    """a mailer is a (from_address, backend) pair"""
+    task_backend = todo_get_backend(task)
+    if task_backend is None:
+        return (None, mail.get_connection)
+
+    from_address = getattr(task_backend, "from_address")
+    from_address = email.utils.formataddr((user.username, from_address))
+    return (from_address, task_backend)
+
+
+def todo_send_mail(user, task, subject, body, recip_list):
+    '''Send an email attached to task, triggered by user'''
+    references = Comment.objects.filter(task=task).only('email_message_id')
+    references = (ref.email_message_id for ref in references)
+    references = ' '.join(filter(bool, references))
+
+    from_address, backend = todo_get_mailer(user, task)
+    message_hash = hash((
+        subject,
+        body,
+        from_address,
+        frozenset(recip_list),
+        references,
+    ))
+
+    message_id = (
+        # the task_id enables attaching back notification answers
+        "<notif-{task_id}."
+        # the message hash / epoch pair enables deduplication
+        "{message_hash:x}."
+        "{epoch}@django-todo>"
+    ).format(
+        task_id=task.pk,
+        # avoid the -hexstring case (hashes can be negative)
+        message_hash=abs(message_hash),
+        epoch=int(time.time())
+    )
+
+    # the thread message id is used as a common denominator between all
+    # notifications for some task. This message doesn't actually exist,
+    # it's just there to make threading possible
+    thread_message_id = "<thread-{}@django-todo>".format(task.pk)
+    references = '{} {}'.format(references, thread_message_id)
+
+    with backend() as connection:
+        message = mail.EmailMessage(
+            subject,
+            body,
+            from_address,
+            recip_list,
+            [], # Bcc
+            headers={
+                **getattr(backend, 'headers', {}),
+                'Message-ID': message_id,
+                'References': references,
+                'In-reply-to': thread_message_id,
+            },
+            connection=connection,
+        )
+        message.send()
+
+
+def send_notify_mail(new_task):
+    '''
+    Send email to assignee if task is assigned to someone other than submittor.
+    Unassigned tasks should not try to notify.
+    '''
+
+    if new_task.assigned_to == new_task.created_by:
+        return
 
-    if not new_task.assigned_to == new_task.created_by:
     current_site = Site.objects.get_current()
-        email_subject = render_to_string("todo/email/assigned_subject.txt", {"task": new_task})
-        email_body = render_to_string(
+    subject = render_to_string("todo/email/assigned_subject.txt", {"task": new_task})
+    body = render_to_string(
         "todo/email/assigned_body.txt", {"task": new_task, "site": current_site}
     )
 
-        send_mail(
-            email_subject,
-            email_body,
-            new_task.created_by.email,
-            [new_task.assigned_to.email],
-            fail_silently=False,
-        )
+    recip_list = [new_task.assigned_to.email]
+    todo_send_mail(new_task.created_by, new_task, subject, body, recip_list)
 
 
 def send_email_to_thread_participants(task, msg_body, user, subject=None):
-    # Notify all previous commentors on a Task about a new comment.
+    '''Notify all previous commentors on a Task about a new comment.'''
 
     current_site = Site.objects.get_current()
-    email_subject = (
-        subject if subject else render_to_string("todo/email/assigned_subject.txt", {"task": task})
+    email_subject = subject
+    if not subject:
+        subject = render_to_string(
+            "todo/email/assigned_subject.txt",
+            {"task": task}
         )
+
     email_body = render_to_string(
         "todo/email/newcomment_body.txt",
         {"task": task, "body": msg_body, "site": current_site, "user": user},
     )
 
-    # Get list of all thread participants - everyone who has commented, plus task creator.
+    # Get all thread participants
     commenters = Comment.objects.filter(task=task)
-    recip_list = [ca.author.email for ca in commenters]
-    recip_list.append(task.created_by.email)
-    recip_list = list(set(recip_list))  # Eliminate duplicates
+    recip_list = set(
+        ca.author.email
+        for ca in commenters
+        if ca.author is not None
+    )
+    for related_user in (task.created_by, task.assigned_to):
+        if related_user is not None:
+            recip_list.add(related_user.email)
+    recip_list = list(m for m in recip_list if m)
 
-    send_mail(email_subject, email_body, task.created_by.email, recip_list, fail_silently=False)
+    todo_send_mail(user, task, email_subject, email_body, recip_list)
 
 
 def toggle_task_completed(task_id: int) -> bool:

todo/views/task_autocomplete.py

@@ -0,0 +1,29 @@
+from dal import autocomplete
+from django.contrib.auth.decorators import login_required
+from django.core.exceptions import PermissionDenied
+from django.shortcuts import get_object_or_404
+from django.utils.decorators import method_decorator
+from todo.models import Task
+from todo.utils import user_can_read_task
+
+
+class TaskAutocomplete(autocomplete.Select2QuerySetView):
+    @method_decorator(login_required)
+    def dispatch(self, request, task_id, *args, **kwargs):
+        self.task = get_object_or_404(Task, pk=task_id)
+        if not user_can_read_task(self.task, request.user):
+            raise PermissionDenied
+
+        return super().dispatch(request, task_id, *args, **kwargs)
+
+    def get_queryset(self):
+        # Don't forget to filter out results depending on the visitor !
+        if not self.request.user.is_authenticated:
+            return Task.objects.none()
+
+        qs = Task.objects.filter(task_list=self.task.task_list).exclude(pk=self.task.pk)
+
+        if self.q:
+            qs = qs.filter(title__istartswith=self.q)
+
+        return qs

todo/views/task_detail.py

@@ -1,33 +1,31 @@
+import bleach
 import datetime
 
-import bleach
+from django import forms
+from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required, user_passes_test
 from django.core.exceptions import PermissionDenied
 from django.http import HttpResponse
-from django.shortcuts import get_object_or_404, redirect, render
+from django.shortcuts import get_object_or_404, redirect, render, redirect
+from django.urls import reverse
+from django.utils.decorators import method_decorator
 
 from todo.forms import AddEditTaskForm
 from todo.models import Comment, Task
-from todo.utils import send_email_to_thread_participants, toggle_task_completed, staff_check
+from todo.utils import send_email_to_thread_participants, toggle_task_completed, staff_check, user_can_read_task
+from todo.features import HAS_TASK_MERGE
 
 
-@login_required
-@user_passes_test(staff_check)
-def task_detail(request, task_id: int) -> HttpResponse:
-    """View task details. Allow task details to be edited. Process new comments on task.
-    """
+if HAS_TASK_MERGE:
+    from dal import autocomplete
+    from todo.views.task_autocomplete import TaskAutocomplete
 
-    task = get_object_or_404(Task, pk=task_id)
-    comment_list = Comment.objects.filter(task=task_id)
 
-    # Ensure user has permission to view task. Admins can view all tasks.
-    # Get the group this task belongs to, and check whether current user is a member of that group.
-    if task.task_list.group not in request.user.groups.all() and not request.user.is_staff:
-        raise PermissionDenied
+def handle_add_comment(request, task):
+    if not request.POST.get("add_comment"):
+        return
 
-    # Save submitted comments
-    if request.POST.get("add_comment"):
     Comment.objects.create(
         author=request.user,
         task=task,
@@ -40,12 +38,68 @@ def task_detail(request, task_id: int) -> HttpResponse:
         request.user,
         subject='New comment posted on task "{}"'.format(task.title),
     )
-        messages.success(request, "Comment posted. Notification email sent to thread participants.")
+
+    messages.success(
+        request, "Comment posted. Notification email sent to thread participants."
+    )
+
+
+@login_required
+@user_passes_test(staff_check)
+def task_detail(request, task_id: int) -> HttpResponse:
+    """View task details. Allow task details to be edited. Process new comments on task.
+    """
+
+    task = get_object_or_404(Task, pk=task_id)
+    comment_list = Comment.objects.filter(task=task_id).order_by('-date')
+
+    # Ensure user has permission to view task. Admins can view all tasks.
+    # Get the group this task belongs to, and check whether current user is a member of that group.
+    if not user_can_read_task(task, request.user):
+        raise PermissionDenied
+
+    # Handle task merging
+    if not HAS_TASK_MERGE:
+        merge_form = None
+    else:
+        class MergeForm(forms.Form):
+            merge_target = forms.ModelChoiceField(
+                queryset=Task.objects.all(),
+                widget=autocomplete.ModelSelect2(
+                    url=reverse("todo:task_autocomplete", kwargs={"task_id": task_id})
+                ),
+            )
+
+        # Handle task merging
+        if not request.POST.get("merge_task_into"):
+            merge_form = MergeForm()
+        else:
+            merge_form = MergeForm(request.POST)
+            if merge_form.is_valid():
+                merge_target = merge_form.cleaned_data["merge_target"]
+            if not user_can_read_task(merge_target, request.user):
+                raise PermissionDenied
+
+            task.merge_into(merge_target)
+            return redirect(reverse(
+                "todo:task_detail",
+                kwargs={"task_id": merge_target.pk}
+            ))
+
+    # Save submitted comments
+    handle_add_comment(request, task)
 
     # Save task edits
-    if request.POST.get("add_edit_task"):
+    if not request.POST.get("add_edit_task"):
         form = AddEditTaskForm(
-            request.user, request.POST, instance=task, initial={"task_list": task.task_list}
+            request.user, instance=task, initial={"task_list": task.task_list}
+        )
+    else:
+        form = AddEditTaskForm(
+            request.user,
+            request.POST,
+            instance=task,
+            initial={"task_list": task.task_list},
         )
 
         if form.is_valid():
@@ -54,10 +108,10 @@ def task_detail(request, task_id: int) -> HttpResponse:
             item.save()
             messages.success(request, "The task has been edited.")
             return redirect(
-                "todo:list_detail", list_id=task.task_list.id, list_slug=task.task_list.slug
+                "todo:list_detail",
+                list_id=task.task_list.id,
+                list_slug=task.task_list.slug,
             )
-    else:
-        form = AddEditTaskForm(request.user, instance=task, initial={"task_list": task.task_list})
 
     # Mark complete
     if request.POST.get("toggle_done"):
@@ -72,6 +126,13 @@ def task_detail(request, task_id: int) -> HttpResponse:
     else:
         thedate = datetime.datetime.now()
 
-    context = {"task": task, "comment_list": comment_list, "form": form, "thedate": thedate}
+    context = {
+        "task": task,
+        "comment_list": comment_list,
+        "form": form,
+        "merge_form": merge_form,
+        "thedate": thedate,
+        "comment_classes": getattr(settings, 'TODO_COMMENT_CLASSES', []),
+    }
 
     return render(request, "todo/task_detail.html", context)