From 46b7ef376431c93d78e2d04f71a91a0f904051ee Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Wed, 4 Mar 2026 16:31:29 +0530 Subject: [PATCH 01/11] feat(email): add backend api for undoing email send --- frappe/core/doctype/communication/email.py | 63 ++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 62e7873c79..a504a95c6b 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -18,7 +18,10 @@ from frappe.utils import ( get_imaginary_pixel_response, get_string_between, list_to_str, + now_datetime, + parse_addr, split_emails, + time_diff_in_seconds, validate_email_address, ) @@ -328,3 +331,63 @@ def update_communication_as_read(name): name, {"read_by_recipient": 1, "delivery_status": "Read", "read_by_recipient_on": get_datetime()}, ) + + +@frappe.whitelist() +def undo_email_send(communication_name: str): + communication = frappe.get_doc("Communication", communication_name) + + if communication.owner != frappe.session.user: + frappe.throw(_("You are not authorized to undo this email")) + + if communication.sent_or_received != "Sent" or communication.communication_medium != "Email": + frappe.throw(_("Failed to delete communication")) + + time_elapsed_in_seconds = time_diff_in_seconds(now_datetime(), communication.creation) + if time_elapsed_in_seconds > 10: + frappe.throw(_("Email undo window is over. Cannot undo email.")) + + email_queue_records_status = frappe.get_all( + "Email Queue", + filters={"communication": communication_name}, + pluck="status", + ) + + for status in email_queue_records_status: + if status != "Not Sent": + frappe.throw(_("It is too late to undo this email. It is already being sent.")) + + frappe.db.delete( + "Email Queue Recipient", + { + "parent": [ + "in", + frappe.get_all("Email Queue", filters={"communication": communication_name}, pluck="name"), + ] + }, + ) + + frappe.db.delete("Email Queue", {"communication": communication_name}) + + communication_data = { + "subject": communication.subject, + "content": communication.content, + "recipients": communication.recipients, + "cc": communication.cc, + "bcc": communication.bcc, + "doc": {"doctype": communication.reference_doctype, "name": communication.reference_name}, + "sender": communication.sender, + "email_template": communication.email_template, + "send_read_receipt": communication.read_receipt, + } + + linked_files = frappe.get_all( + "File", + filters={"attached_to_doctype": "Communication", "attached_to_name": communication_name}, + pluck="name", + ) + communication_data["attachments"] = linked_files + + communication.delete(ignore_permissions=True) + + return communication_data From 5f4174be2d2c9730e4c4b9c4967a5ffce020bc11 Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Wed, 4 Mar 2026 16:34:57 +0530 Subject: [PATCH 02/11] feat(email): delay email queue flushing for recording created between now and 10 seconds --- frappe/email/queue.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 6e47541385..67a13479f6 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -5,7 +5,7 @@ from datetime import timedelta import frappe from frappe import _, msgprint from frappe.utils import cint, cstr, get_url, now_datetime -from frappe.utils.data import getdate +from frappe.utils.data import add_to_date, getdate from frappe.utils.verified_command import get_signed_params, verify_request # After this percent of failures in every batch, entire batch is aborted. @@ -163,6 +163,7 @@ def flush(): def get_queue(): batch_size = cint(frappe.conf.email_queue_batch_size) or 500 + undo_window = add_to_date(now_datetime(seconds=-10)) return frappe.db.sql( f"""select @@ -171,11 +172,12 @@ def get_queue(): `tabEmail Queue` where (status='Not Sent' or status='Partially Sent') and - (send_after is null or send_after < %(now)s) + (send_after is null or send_after < %(now)s) and + (creation < %(undo_window)s) order by priority desc, retry asc, creation asc limit {batch_size}""", - {"now": now_datetime()}, + {"now": now_datetime(), "undo_window": undo_window}, as_dict=True, ) From d38531b9558aeb067ad85b5695a4d3047424e2e1 Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Wed, 4 Mar 2026 16:36:50 +0530 Subject: [PATCH 03/11] feat(email): show undo email alert for 10 seconds after sending email and reopen email composer with communication data --- .../public/js/frappe/views/communication.js | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index 528b5f81a3..bc6f4a6013 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -896,6 +896,8 @@ frappe.views.CommunicationComposer = class { if (!r.exc) { frappe.utils.play_sound("email"); + const communication_name = r.message["name"]; + if (r.message["emails_not_sent_to"]) { frappe.msgprint( __("Email not sent to {0} (unsubscribed / disabled)", [ @@ -910,6 +912,61 @@ frappe.views.CommunicationComposer = class { me.frm.reload_doc(); } + // Show undo toast for 10 seconds + let undo_alert = frappe.show_alert( + { + message: `${__( + "Email Sent" + )} ${__( + "Undo" + )}`, + indicator: "green", + }, + 10, + { + undo: () => { + if (undo_alert) { + undo_alert.find(".close").click(); + } + frappe.call({ + method: "frappe.core.doctype.communication.email.undo_email_send", + args: { communication_name: communication_name }, + callback(undo_r) { + if (!undo_r.exc) { + if (me.frm) { + me.frm.reload_doc(); + } + + // Reopen the composer with the recovered data + const d = undo_r.message; + new frappe.views.CommunicationComposer({ + doc: d.doc, + subject: d.subject, + recipients: d.recipients, + cc: d.cc, + bcc: d.bcc, + message: d.content, + sender: d.sender, + email_template: d.email_template, + read_receipt: d.send_read_receipt, + attachments: d.attachments, + frm: me.frm, + }); + + // Show alert after modal so it stays prominent + setTimeout(() => { + frappe.show_alert({ + message: __("Email sending undone"), + indicator: "blue", + }); + }, 200); + } + }, + }); + }, + } + ); + // try the success callback if it exists if (me.success) { try { From 007bef93091638704fd7905ebedee57e802c0df1 Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Wed, 4 Mar 2026 16:37:58 +0530 Subject: [PATCH 04/11] refactor: increase toast z index to render above email composer modal --- frappe/public/scss/desk/toast.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/scss/desk/toast.scss b/frappe/public/scss/desk/toast.scss index 695c7118b7..36b067a5d3 100644 --- a/frappe/public/scss/desk/toast.scss +++ b/frappe/public/scss/desk/toast.scss @@ -2,7 +2,7 @@ position: fixed; bottom: 0px; right: 20px; - z-index: 1050; + z-index: 2000; @include media-breakpoint-down(sm) { right: 0; From c02cfc3aed2cc696181260ae17199cf6576e12ce Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Wed, 4 Mar 2026 16:55:36 +0530 Subject: [PATCH 05/11] feat(email): add tests for undo email send --- .../communication/test_communication.py | 83 ++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index 48a6e9b636..aa349aaa30 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -1,12 +1,14 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +from datetime import timedelta from typing import TYPE_CHECKING import frappe from frappe.core.doctype.communication.communication import Communication, get_emails, parse_email -from frappe.core.doctype.communication.email import add_attachments, make +from frappe.core.doctype.communication.email import add_attachments, make, undo_email_send from frappe.email.doctype.email_queue.email_queue import EmailQueue from frappe.tests import IntegrationTestCase +from frappe.utils import now_datetime if TYPE_CHECKING: from frappe.contacts.doctype.contact.contact import Contact @@ -438,6 +440,85 @@ class TestCommunicationEmailMixin(IntegrationTestCase): self.assertEqual(attached_file.file_name, file_name) self.assertEqual(attached_file.get_content(), file_content) + def test_undo_email_send(self): + """Undo should delete Communication and Email Queue, and return original data.""" + comm = self.new_communication(recipients=["to@test.com"]) + comm.sent_or_received = "Sent" + comm.save(ignore_permissions=True) + + eq = frappe.get_doc( + { + "doctype": "Email Queue", + "sender": "Test ", + "message": "Test message", + "status": "Not Sent", + "priority": 1, + "communication": comm.name, + "recipients": [{"recipient": "to@test.com", "status": "Not Sent"}], + } + ).insert(ignore_permissions=True) + + result = undo_email_send(comm.name) + + self.assertFalse(frappe.db.exists("Communication", comm.name)) + self.assertFalse(frappe.db.exists("Email Queue", eq.name)) + self.assertFalse(frappe.db.exists("Email Queue Recipient", {"parent": eq.name})) + self.assertEqual(result["subject"], comm.subject) + self.assertEqual(result["recipients"], comm.recipients) + + def test_undo_email_send_fails_for_different_user(self): + """Undo should fail if the current user is not the owner.""" + comm = self.new_communication(recipients=["to@test.com"]) + comm.sent_or_received = "Sent" + comm.save(ignore_permissions=True) + frappe.db.set_value("Communication", comm.name, "owner", "other@test.com") + + with self.assertRaises(frappe.exceptions.ValidationError): + undo_email_send(comm.name) + + self.assertTrue(frappe.db.exists("Communication", comm.name)) + + def test_undo_email_send_fails_after_time_window(self): + """Undo should fail if the 10-second window has passed.""" + comm = self.new_communication(recipients=["to@test.com"]) + comm.sent_or_received = "Sent" + comm.save(ignore_permissions=True) + frappe.db.set_value( + "Communication", + comm.name, + "creation", + now_datetime() - timedelta(seconds=15), + update_modified=False, + ) + + with self.assertRaises(frappe.exceptions.ValidationError): + undo_email_send(comm.name) + + self.assertTrue(frappe.db.exists("Communication", comm.name)) + + def test_undo_email_send_fails_if_already_sent(self): + """Undo should fail if Email Queue status is not 'Not Sent'.""" + comm = self.new_communication(recipients=["to@test.com"]) + comm.sent_or_received = "Sent" + comm.save(ignore_permissions=True) + + frappe.get_doc( + { + "doctype": "Email Queue", + "sender": "Test ", + "message": "Test message", + "status": "Sent", + "priority": 1, + "communication": comm.name, + "recipients": [{"recipient": "to@test.com", "status": "Sent"}], + } + ).insert(ignore_permissions=True) + + with self.assertRaises(frappe.exceptions.ValidationError): + undo_email_send(comm.name) + + self.assertTrue(frappe.db.exists("Communication", comm.name)) + def create_email_account() -> "EmailAccount": frappe.delete_doc_if_exists("Email Account", "_Test Comm Account 1") From 2e1aa18e0c4515fa7f193f0a256770c82155fbda Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Wed, 4 Mar 2026 17:21:11 +0530 Subject: [PATCH 06/11] refactor: improve undo button positioning --- frappe/public/js/frappe/views/communication.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index bc6f4a6013..2bcd60d0d1 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -915,11 +915,14 @@ frappe.views.CommunicationComposer = class { // Show undo toast for 10 seconds let undo_alert = frappe.show_alert( { - message: `${__( - "Email Sent" - )} ${__( - "Undo" - )}`, + message: ` +
+ ${__("Email Sent")} + +
+ `, indicator: "green", }, 10, From 55de855b324b7b6ce194ad49dd6323627cc07db8 Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Wed, 4 Mar 2026 17:44:09 +0530 Subject: [PATCH 07/11] fix: correct add_to_date function call and parameterize batch size in get_queue sql query --- frappe/email/queue.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 67a13479f6..e0aa112e60 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -163,10 +163,10 @@ def flush(): def get_queue(): batch_size = cint(frappe.conf.email_queue_batch_size) or 500 - undo_window = add_to_date(now_datetime(seconds=-10)) + undo_window = add_to_date(now_datetime(), seconds=-10) return frappe.db.sql( - f"""select + """select name, sender from `tabEmail Queue` @@ -176,8 +176,8 @@ def get_queue(): (creation < %(undo_window)s) order by priority desc, retry asc, creation asc - limit {batch_size}""", - {"now": now_datetime(), "undo_window": undo_window}, + limit %(batch_size)s""", + {"now": now_datetime(), "undo_window": undo_window, "batch_size": batch_size}, as_dict=True, ) From ab6806a876934c076cf4fa3d23e9591751fc87a9 Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Thu, 5 Mar 2026 12:43:02 +0530 Subject: [PATCH 08/11] test: modify existing flush and send_after email tests to add delay to flush --- frappe/tests/test_email.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_email.py b/frappe/tests/test_email.py index 2e3932c7e3..4a2ec50c30 100644 --- a/frappe/tests/test_email.py +++ b/frappe/tests/test_email.py @@ -57,16 +57,20 @@ class TestEmail(IntegrationTestCase): def test_send_after(self): self.test_email_queue(send_after=1) from frappe.email.queue import flush + from frappe.utils import add_to_date, now_datetime - flush() + with self.freeze_time(add_to_date(now_datetime(), seconds=12)): + flush() email_queue = frappe.db.sql("""select name from `tabEmail Queue` where status='Sent'""", as_dict=1) self.assertEqual(len(email_queue), 0) def test_flush(self): self.test_email_queue() from frappe.email.queue import flush + from frappe.utils import add_to_date, now_datetime - flush() + with self.freeze_time(add_to_date(now_datetime(), seconds=12)): + flush() email_queue = frappe.db.sql("""select name from `tabEmail Queue` where status='Sent'""", as_dict=1) self.assertEqual(len(email_queue), 1) queue_recipients = [ From bfa13290953b848cd3ea5f315a4dfce36c62eb0d Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Thu, 5 Mar 2026 12:46:16 +0530 Subject: [PATCH 09/11] refactor: improve undo button ui in toast --- frappe/public/js/frappe/views/communication.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index 2bcd60d0d1..f8dec50e7a 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -912,15 +912,14 @@ frappe.views.CommunicationComposer = class { me.frm.reload_doc(); } - // Show undo toast for 10 seconds let undo_alert = frappe.show_alert( { message: `
${__("Email Sent")} - +
`, indicator: "green", @@ -956,7 +955,6 @@ frappe.views.CommunicationComposer = class { frm: me.frm, }); - // Show alert after modal so it stays prominent setTimeout(() => { frappe.show_alert({ message: __("Email sending undone"), From 702dd7913f008db8d3c229e1604ed440a03010c3 Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Thu, 5 Mar 2026 13:00:33 +0530 Subject: [PATCH 10/11] refactor: remove unnecessary set timeout on email undone toast --- frappe/public/js/frappe/views/communication.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index f8dec50e7a..708a5edd7a 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -955,12 +955,10 @@ frappe.views.CommunicationComposer = class { frm: me.frm, }); - setTimeout(() => { - frappe.show_alert({ - message: __("Email sending undone"), - indicator: "blue", - }); - }, 200); + frappe.show_alert({ + message: __("Email sending undone"), + indicator: "blue", + }); } }, }); From c316251a28f07edfd6c91d792f3a5cd03ebb4022 Mon Sep 17 00:00:00 2001 From: Shrihari Mahabal Date: Tue, 10 Mar 2026 10:53:29 +0530 Subject: [PATCH 11/11] refactor: modify requested changes --- frappe/core/doctype/communication/email.py | 40 +++++------ .../communication/test_communication.py | 14 ++-- .../public/js/frappe/views/communication.js | 72 +++++++++---------- 3 files changed, 57 insertions(+), 69 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index a504a95c6b..733a5d73dc 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -345,29 +345,25 @@ def undo_email_send(communication_name: str): time_elapsed_in_seconds = time_diff_in_seconds(now_datetime(), communication.creation) if time_elapsed_in_seconds > 10: - frappe.throw(_("Email undo window is over. Cannot undo email.")) + frappe.msgprint( + _("Email undo window is over. Cannot undo email."), alert=True, indicator="red", raise_exception=1 + ) - email_queue_records_status = frappe.get_all( - "Email Queue", - filters={"communication": communication_name}, - pluck="status", + email_queue_records = frappe.get_all( + "Email Queue", filters={"communication": communication_name}, fields=["name", "status"] ) - for status in email_queue_records_status: - if status != "Not Sent": - frappe.throw(_("It is too late to undo this email. It is already being sent.")) + for queue in email_queue_records: + if queue.status != "Not Sent": + frappe.msgprint( + _("It is too late to undo this email. It is already being sent."), + alert=True, + indicator="red", + raise_exception=1, + ) - frappe.db.delete( - "Email Queue Recipient", - { - "parent": [ - "in", - frappe.get_all("Email Queue", filters={"communication": communication_name}, pluck="name"), - ] - }, - ) - - frappe.db.delete("Email Queue", {"communication": communication_name}) + for queue in email_queue_records: + frappe.delete_doc("Email Queue", queue.name, ignore_permissions=True) communication_data = { "subject": communication.subject, @@ -377,7 +373,6 @@ def undo_email_send(communication_name: str): "bcc": communication.bcc, "doc": {"doctype": communication.reference_doctype, "name": communication.reference_name}, "sender": communication.sender, - "email_template": communication.email_template, "send_read_receipt": communication.read_receipt, } @@ -386,6 +381,11 @@ def undo_email_send(communication_name: str): filters={"attached_to_doctype": "Communication", "attached_to_name": communication_name}, pluck="name", ) + + if linked_files: + for file_name in linked_files: + frappe.db.set_value("File", file_name, {"attached_to_doctype": None, "attached_to_name": None}) + communication_data["attachments"] = linked_files communication.delete(ignore_permissions=True) diff --git a/frappe/core/doctype/communication/test_communication.py b/frappe/core/doctype/communication/test_communication.py index aa349aaa30..b7ad8e2c19 100644 --- a/frappe/core/doctype/communication/test_communication.py +++ b/frappe/core/doctype/communication/test_communication.py @@ -8,7 +8,7 @@ from frappe.core.doctype.communication.communication import Communication, get_e from frappe.core.doctype.communication.email import add_attachments, make, undo_email_send from frappe.email.doctype.email_queue.email_queue import EmailQueue from frappe.tests import IntegrationTestCase -from frappe.utils import now_datetime +from frappe.utils import add_to_date, now_datetime if TYPE_CHECKING: from frappe.contacts.doctype.contact.contact import Contact @@ -483,16 +483,10 @@ class TestCommunicationEmailMixin(IntegrationTestCase): comm = self.new_communication(recipients=["to@test.com"]) comm.sent_or_received = "Sent" comm.save(ignore_permissions=True) - frappe.db.set_value( - "Communication", - comm.name, - "creation", - now_datetime() - timedelta(seconds=15), - update_modified=False, - ) - with self.assertRaises(frappe.exceptions.ValidationError): - undo_email_send(comm.name) + with self.freeze_time(add_to_date(now_datetime(), seconds=12)): + with self.assertRaises(frappe.exceptions.ValidationError): + undo_email_send(comm.name) self.assertTrue(frappe.db.exists("Communication", comm.name)) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index 708a5edd7a..8424473c30 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -914,14 +914,11 @@ frappe.views.CommunicationComposer = class { let undo_alert = frappe.show_alert( { - message: ` -
- ${__("Email Sent")} - - ${__("Undo")} - -
- `, + message: `${__( + "Email Sent" + )}${__( + "Undo" + )}`, indicator: "green", }, 10, @@ -930,38 +927,35 @@ frappe.views.CommunicationComposer = class { if (undo_alert) { undo_alert.find(".close").click(); } - frappe.call({ - method: "frappe.core.doctype.communication.email.undo_email_send", - args: { communication_name: communication_name }, - callback(undo_r) { - if (!undo_r.exc) { - if (me.frm) { - me.frm.reload_doc(); - } - - // Reopen the composer with the recovered data - const d = undo_r.message; - new frappe.views.CommunicationComposer({ - doc: d.doc, - subject: d.subject, - recipients: d.recipients, - cc: d.cc, - bcc: d.bcc, - message: d.content, - sender: d.sender, - email_template: d.email_template, - read_receipt: d.send_read_receipt, - attachments: d.attachments, - frm: me.frm, - }); - - frappe.show_alert({ - message: __("Email sending undone"), - indicator: "blue", - }); + frappe + .xcall( + "frappe.core.doctype.communication.email.undo_email_send", + { communication_name: communication_name } + ) + .then((d) => { + if (me.frm) { + me.frm.reload_doc(); } - }, - }); + + // Reopen the composer with the recovered data + new frappe.views.CommunicationComposer({ + doc: d.doc, + subject: d.subject, + recipients: d.recipients, + cc: d.cc, + bcc: d.bcc, + message: d.content, + sender: d.sender, + read_receipt: d.send_read_receipt, + attachments: d.attachments, + frm: me.frm, + }); + + frappe.show_alert({ + message: __("Email sending undone"), + indicator: "blue", + }); + }); }, } );