From c875fbe20a1da5f504ad25040a24af64a5e30a8b Mon Sep 17 00:00:00 2001 From: phot0n Date: Fri, 8 Dec 2023 17:36:54 +0530 Subject: [PATCH 1/2] refactor: email bulk retry * move msgprint to frontend so that it cna be displayed before the api call is completed * use set_value instead of save() doc method so that other doc values can't be altered --- frappe/email/doctype/email_queue/email_queue.py | 10 ++++------ frappe/email/doctype/email_queue/email_queue_list.js | 7 +++++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index 8f02a6e5f2..129e2be2c9 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -441,7 +441,8 @@ class SendMailContext: @frappe.whitelist() def bulk_retry(queues): - frappe.only_for("System Manager") + if not frappe.has_permission("Email Queue", throw=True): + return if isinstance(queues, str): queues = json.loads(queues) @@ -449,11 +450,8 @@ def bulk_retry(queues): if not queues: return - frappe.msgprint( - _("Updating Email Queue Statuses. The emails will be picked up in the next scheduled run."), - _("Processing..."), - ) - + # NOTE: this will probably work fine with the way current listview works (showing and selecting 20-20 records) + # but, ideally this should be enqueued email_queue = frappe.qb.DocType("Email Queue") frappe.qb.update(email_queue).set(email_queue.status, "Not Sent").set(email_queue.modified, now()).set( email_queue.modified_by, frappe.session.user diff --git a/frappe/email/doctype/email_queue/email_queue_list.js b/frappe/email/doctype/email_queue/email_queue_list.js index fde7ddf7f3..4058d193fa 100644 --- a/frappe/email/doctype/email_queue/email_queue_list.js +++ b/frappe/email/doctype/email_queue/email_queue_list.js @@ -45,6 +45,13 @@ function show_toggle_sending_button(list_view) { function add_bulk_retry_button_to_actions(list_view) { list_view.page.add_actions_menu_item(__("Retry Sending"), () => { + frappe.msgprint( + __( + "Updating Email Queue Statuses. The emails will be picked up in the next scheduled run." + ), + __("Processing...") + ); + frappe.call({ method: "frappe.email.doctype.email_queue.email_queue.bulk_retry", args: { From 082549e273edc2f026949160264fadcc32ce28e1 Mon Sep 17 00:00:00 2001 From: phot0n Date: Mon, 11 Dec 2023 20:58:02 +0530 Subject: [PATCH 2/2] refactor: use the same method for retrying one as well as multiple email queues * removed retry_sending method from email queue controller --- frappe/email/doctype/email_queue/email_queue.js | 11 ++++++++--- frappe/email/doctype/email_queue/email_queue.py | 8 +------- frappe/email/doctype/email_queue/email_queue_list.js | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/frappe/email/doctype/email_queue/email_queue.js b/frappe/email/doctype/email_queue/email_queue.js index b9a24342ba..f45b855f95 100644 --- a/frappe/email/doctype/email_queue/email_queue.js +++ b/frappe/email/doctype/email_queue/email_queue.js @@ -19,13 +19,18 @@ frappe.ui.form.on("Email Queue", { } else if (frm.doc.status == "Error") { frm.add_custom_button("Retry Sending", function () { frm.call({ - method: "retry_sending", - doc: frm.doc, + method: "frappe.email.doctype.email_queue.email_queue.retry_sending", args: { - name: frm.doc.name, + queues: [frm.doc.name], }, callback: function () { frm.reload_doc(); + frappe.show_alert({ + message: __( + "Status Updated. The email will be picked up in the next scheduled run." + ), + indicator: "green", + }); }, }); }); diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index 129e2be2c9..d42c3ce97e 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -222,12 +222,6 @@ class EmailQueue(Document): .where(email_recipient.creation < (Now() - Interval(days=days))) ).run() - @frappe.whitelist() - def retry_sending(self): - if self.status == "Error": - self.status = "Not Sent" - self.save(ignore_permissions=True) - @task(queue="short") @deprecated @@ -440,7 +434,7 @@ class SendMailContext: @frappe.whitelist() -def bulk_retry(queues): +def retry_sending(queues: str | list[str]): if not frappe.has_permission("Email Queue", throw=True): return diff --git a/frappe/email/doctype/email_queue/email_queue_list.js b/frappe/email/doctype/email_queue/email_queue_list.js index 4058d193fa..45d606777e 100644 --- a/frappe/email/doctype/email_queue/email_queue_list.js +++ b/frappe/email/doctype/email_queue/email_queue_list.js @@ -53,7 +53,7 @@ function add_bulk_retry_button_to_actions(list_view) { ); frappe.call({ - method: "frappe.email.doctype.email_queue.email_queue.bulk_retry", + method: "frappe.email.doctype.email_queue.email_queue.retry_sending", args: { queues: list_view.get_checked_items(true), },