move message_id threading to communication to work with inbox #2458
Conversation
|
missed a line all good |
|
@RobertSchouten rebase required |
| if references: | ||
| if "notification" in references: | ||
| communication.unread_notification_sent = 1 | ||
|
|
There was a problem hiding this comment.
allows direct sending of emails to erpnext systems without response emails being sent preventing infinte loops (though there is still validatation preventing sending to an internal account removed in inbox)
There was a problem hiding this comment.
I don't see a link yet, so maybe it comes in the next PR?
There was a problem hiding this comment.
notification is added to header here
https://github.com/frappe/frappe/pull/2458/files#diff-a95ae98f6737046bfb40cb1fd19689f8R191
so if email is directly recieved by an erpnext server it set's it so the server believes notifcations are sent
|
|
||
| last_mail = frappe.get_doc('Email Queue', dict(reference_name=event.name)) | ||
| last_mail = frappe.get_doc('Communication', dict(reference_name=event.name)) | ||
|
|
There was a problem hiding this comment.
make a separate test case for communication?
There was a problem hiding this comment.
@rmehta with this change threading only occurs on messages sent from communications
though ive yet to see an email send from frappe.sendmail that should b included in a thread
There was a problem hiding this comment.
in "Daily Work Summary" threading is by email queue as there are no communications for sent emails
There was a problem hiding this comment.
@rmehta modified daily work summary to send a communication
frappe/erpnext#7233
There was a problem hiding this comment.
Why not allow to thread via Email Queue too? Any harm?
RobertSchouten
force-pushed
the
message_id_dev
branch
from
c198503
to
4ccd119
Compare
moves thread matching by message_id to communcation to avoid issues with the upcoming email inbox
please merge before 7.2 to prevent patching