Mailtracker issues

[ezzra]

first, thank you very much for the new mailtracking feature, I just wanted to start working on such a feature :)

But some issues came up for me:

• had to change Comment.email_message_id to CharField and remake migrations, because mysql cannot use TextField as key and throws an error. TextField does not make sense here by the way?!
• preserve=false setting does not work for me, emails are not deleted
• the TODO_MAIL_BACKENDS first confused me, until I realised that they are not necessary for the mailtracking feature, its just that you can now have seperate email configs for each list, correct?
• when a new task is created, why is he mailbody posted as comment? Is this intended? I think its good to save it as task description
• at the the moment, anyone can use the feature when the email address is known. This should be limited to the user email addresses, even if emailaddresses can be spoofed. This way the messages could be just connected to the account and not to blank emailadresses

[ezzra]

I will start working on that last issue and connect mailaddresses to accounts..

[multun]

Heyo,

I had to change Comment.email_message_id to CharField and remake migrations, because mysql cannot use TextField as key and throws an error. TextField does not make sense here by the way?!

Ooch, this is worrying. I wasn't aware of this limitation, and I'm not sure about how it should be handled :/ TextField does makes sense because message-ids have no size limitations, and actually, postgresql doesn't make much of a difference between text and charfields.

https://stackoverflow.com/questions/33211540/is-there-anyway-to-create-unique-textfield-in-django-with-mysql-db-backend

What fix would you suggest ?

the TODO_MAIL_BACKENDS first confused me, until I realised that they are not necessary for the mailtracking feature, its just that you can now have seperate email configs for each list, correct?

That's it

preserve=false setting does not work for me, emails are not deleted

I guess it needs a separate issue and further diagnosis. It's probably due to an unfortunate regression during development, or IMAP madness

when a new task is created, why is he mailbody posted as comment? Is this intended? I think its good to save it as task description

I know that's weird, but that's absolutely intended !

1. it keeps the original message read-only
2. avoids an ugly task_message_id thing
3. enables commenting the mail task without interfering with the original message

at the the moment, anyone can use the feature when the email address is known. This should be limited to the user email addresses, even if emailaddresses can be spoofed. This way the messages could be just connected to the account and not to blank emailadresses

If you add end up implementing this, can you add the corresponding setting ?

It was implemented this way for multiple reasons:

• it was designed as a customer support sort of thing, where all email answers need to be tracked read-only, and be obviously displayed as email answers
• attaching mail to django users felt cringe because of the low binding between the two, and brought little apparent benefit
• hiding the source email felt like hiding the domain name when browsing

Thanks for testing !

[multun]

@ezzra see #53

[ezzra]

@multun Iam just diving into the tracker.py code, and Iam very confused why you have installed this complex nested system of related_messages and answer_thread and best_task and so on, why didn't you just take the In-Reply-To which is build with the task_id in utils.py ? It looks like you are planning something with nested comments and discussion thread ?!

[multun]

@ezzra that's sort of it ! this code is about properly handling mail threads. It's not used to create a mail tree inside django-todo, but it just takes in account the tree-like structure of mail threads when considering where incoming emails should go to.

There are two separate things going on here:

• related_messages and best_task are about finding the most relevant task to attach the message to. Using only In-Reply-To isn't acceptable because it only references the last email in the chain !
  What if someone responds to a thread, no CC-ing the tracker anymore, and someone answers the non-CCed message, adding back the tracker ? If you parse references, you're going to be able to attach that message back, not using In-Reply-To
• answer_thread, when it exists, it used to attach notification answers

this complex nested system

A full mail threading algorithm looks like that. We aren't anywhere close, fortunately. tracker.py is just a glorified max(len(task.message_ids.intersection(references)) for task in tasks)

[ezzra]

Using only In-Reply-To isn't acceptable because it only references the last email in the chain !
What if someone responds to a thread, no CC-ing the tracker anymore, and someone answers the non-CCed message, adding back the tracker ?

I still don't see the problem or any benefit here. Maybe I don't understand your workflow. In the end, regardless if you take answer_thread or best_task, its just a Task object that will be connected to the new comment. First I do not understand the confusing wording here, but when the track_id is all we need, and its within the header, why not just use it?

I expect users just to answer to the comment notification, which will send an email to the server with something like In-Reply-To: <notif-19.164355fcc835ad02.1532310848@django-todo> in the header. Why not just use this notif-19 and add the comment to Task 19? Do you expect, that users first forward the email somewhere else, or reply to someone else before replying back to server? I do not see where the id could be lost when its just answering to the notification mail.

[multun]

Maybe I don't understand your workflow

I think so, but I'll do my best to make it clearer.

when the track_id is all we need, and its within the header, why not just use it?

Because it's not always within the headers. The task_id is only there when answering an email notification. If the tracker is suscribed to a mailing list, all threads will be separate tasks, each answer being a comment. In this case, there are no notifications involved at all !

I expect users just to answer to the comment notification, which will send an email to the server with something like In-Reply-To: <notif-19.164355fcc835ad02.1532310848@django-todo> in the header.

That's not the only usecase: somebody may send an email to the tracker, get no notification at all (just like your regular contact@company.com), somebody may answer that same email, and as long as the tracker gets the answer too, it'll get attached back to the task.

The mail tracker isn't just about enabling email notification answers. It's about tracking any email. If you want to add knobs to restrict the scope of what it can do, please do, it sounds like a useful thing !

[ezzra]

I see that I have a different usecase at all for the todo tool. I wont use it in way as a client helpdesk, more like the github issue system and the email notification and answering feature is just an easy way so people do not have to login before adding a task or reacting to comments. And I want to keep it simple, thats why I chose the tool in the first way. So I guess I will need to fork this for my own approaches.

[multun]

@ezzra I would suggest adding a feature flag to make it fit what you want. The patch probably is under 10 lines, and the existing code was designed to be very modular.

The internet doesn't need one more fork, the simple way is the unified way.

If you create one more fork:

• you won't benefit from security fixes like this or that
• you won't benefit from improvements of the shared codebase
• code won't go through as much review as with a common project (probably the most important item)

I wont use it in way as a client helpdesk, more like the github issue system and the email notification and answering feature is just an easy way so people do not have to login before adding a task or reacting to comments.

The existing code can already do that, and since comments can't be edited for now, being logged in doesn't change anything.

I guess you miss the following features:

• match the source email to django user emails (1 line of code)
• reject the email if no user is found (2 to 5 lines of code)

The existing code just needs the few lines of glue required to do that. If you don't want to plug into the existing tracker thing, well, you can create another consumer and use it ! Just add a file in there, and here you go.

All the added features are optionnal, and I did my best to keep it simple. I know this feature isn't for everybody. That's why if you don't do anything, no (almost?) additionnal code runs.

[shacker]

@ezzra - @multun and I had quite a bit of back-and-forth during code review about the same issue you raised - that anyone knowing the email addr can successfully post a task or comment into the system, and the spam possibilities that arise from that. I was very reluctant to allow that to happen.

Multun convinced me that, in a way, the "support" queue is really just shadowing an email address, and anyone can already write to any email address on the internet without proving themselves first. And I get the need for a feature like this to exist in an org doing public support, especially with multiple people doing support / sharing a mailbox.

That said, I can see the need to add features/settings for disabling this aspect of the system, or for blocking spammy sender addresses (but beware - it would be all too easy to get into the zillion hassles of email spam handling).

I think a specific issue for that feature should be opened if you feel you need it, rather than this generic "Mailtracker issues" thread.

Also note that todo has long supported the general concept of letting unauthenticated users post support issues. This Mailtracker feature is much more than that. Important to keep their use cases conceptually separated.

[ezzra]

Of course Iam totally aware of the disadvantages of forking, but I do depend on the advantages.

However, it looks like you can force a duplicate entry error for UNIQUE | task_id, email_message_id if you send twice an email with the same message-id for the same task, just fyi

edit: ok should not be true, got an error once but did not try to find out where it comes from, should actually not happen because of get_or_create()

[shacker]

@ezzra A PR is in and almost merged for the MySQL compatibility issue. Based on the discussion above, are there any other items from your original list that you feel still need to be addressed? (I'll want to make a separate ticket for each, or you can). If not, I'd like to close this. Thanks for the helpful feedback!

[shacker]

Closing this for now, but would love to hear confirmation from your or another mysql user that all is well with the 2.3 upgrade on mysql.

And feel free to open individual tickets if any of your issues in the OP still need addressing.

Contributions always welcome!