From d68e46d2cc5da3017a5b3924b9621380160198a6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 19 Jul 2023 16:28:01 +0530 Subject: [PATCH 1/5] fix: disable workflow emails by default make it opt-in instead. --- frappe/workflow/doctype/workflow/workflow.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/workflow/doctype/workflow/workflow.json b/frappe/workflow/doctype/workflow/workflow.json index 4b945a33c22..bddf8a66d79 100644 --- a/frappe/workflow/doctype/workflow/workflow.json +++ b/frappe/workflow/doctype/workflow/workflow.json @@ -54,7 +54,7 @@ "label": "Don't Override Status" }, { - "default": "1", + "default": "0", "description": "Emails will be sent with next possible workflow actions", "fieldname": "send_email_alert", "fieldtype": "Check", From 892c5e30a2804770d0cacd7660afa463bf103b1f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 19 Jul 2023 16:31:00 +0530 Subject: [PATCH 2/5] refactor: Simpler workflow caching Entire document is cached, so no need to create another layer of cache. --- frappe/model/workflow.py | 6 +----- frappe/workflow/doctype/workflow/workflow.py | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/frappe/model/workflow.py b/frappe/model/workflow.py index 0e345a65153..0d7ce13d953 100644 --- a/frappe/model/workflow.py +++ b/frappe/model/workflow.py @@ -228,11 +228,7 @@ def send_email_alert(workflow_name): def get_workflow_field_value(workflow_name, field): - value = frappe.cache.hget("workflow_" + workflow_name, field) - if value is None: - value = frappe.db.get_value("Workflow", workflow_name, field) - frappe.cache.hset("workflow_" + workflow_name, field, value) - return value + return frappe.get_cached_value("Workflow", workflow_name, field) @frappe.whitelist() diff --git a/frappe/workflow/doctype/workflow/workflow.py b/frappe/workflow/doctype/workflow/workflow.py index 56c17261b7c..c28d5e4cd9f 100644 --- a/frappe/workflow/doctype/workflow/workflow.py +++ b/frappe/workflow/doctype/workflow/workflow.py @@ -17,7 +17,6 @@ def validate(self): def on_update(self): self.update_doc_status() frappe.clear_cache(doctype=self.document_type) - frappe.cache.delete_key("workflow_" + self.name) # clear cache created in model/workflow.py def create_custom_field_for_workflow_state(self): frappe.clear_cache(doctype=self.document_type) From 06a905f6008a9c67129ed03d13fde3ccbba97bee Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 19 Jul 2023 17:44:38 +0530 Subject: [PATCH 3/5] fix: disable list view updates during bulk approval Also defer document refreshes by up to 5 seconds. Rarely anyone needs truly realtime list view updates. It's better batch them over at least 5 or so seconds if there's a high volume of changes. --- frappe/public/js/frappe/list/list_view.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index f3af9bd1beb..4d701fe55eb 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -136,7 +136,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { this.workflow_action_items = {}; const actions = this.actions_menu_items.concat(this.workflow_action_menu_items); - actions.map((item) => { + actions.forEach((item) => { const $item = this.page.add_actions_menu_item(item.label, item.action, item.standard); if (item.class) { $item.addClass(item.class); @@ -1669,6 +1669,8 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { get_workflow_action_menu_items() { const workflow_actions = []; + const me = this; + if (frappe.model.has_workflow(this.doctype)) { const actions = frappe.workflow.get_all_transition_actions(this.doctype); actions.forEach((action) => { @@ -1676,11 +1678,16 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { label: __(action), name: action, action: () => { - frappe.xcall("frappe.model.workflow.bulk_workflow_approval", { - docnames: this.get_checked_items(true), - doctype: this.doctype, - action: action, - }); + me.disable_list_update = true; + frappe + .xcall("frappe.model.workflow.bulk_workflow_approval", { + docnames: this.get_checked_items(true), + doctype: this.doctype, + action: action, + }) + .finally(() => { + me.disable_list_update = false; + }); }, is_workflow_action: true, }); From 948b24ee04ab5e54e448aaa4e59134618435cfb8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 19 Jul 2023 18:07:05 +0530 Subject: [PATCH 4/5] fix: duplicate on_row_checked call --- frappe/public/js/frappe/list/list_view.js | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 4d701fe55eb..4facb9893bf 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -593,7 +593,6 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { render() { this.render_list(); this.set_rows_as_checked(); - this.on_row_checked(); this.render_count(); } From c90374c286e29758c399f33be9d1576f88b1d06c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 20 Jul 2023 16:38:26 +0530 Subject: [PATCH 5/5] perf: Lazily compute common workflow transitions --- frappe/public/js/frappe/list/list_view.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 4facb9893bf..dcca1e97846 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -545,7 +545,6 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { if (toggle) { this.page.show_actions_menu(); this.page.clear_primary_action(); - this.toggle_workflow_actions(); } else { this.page.hide_actions_menu(); this.set_primary_action(); @@ -1313,6 +1312,11 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { this.update_checkbox($target); }); + + let me = this; + this.page.actions_btn_group.on("show.bs.dropdown", () => { + me.toggle_workflow_actions(); + }); } setup_like() { @@ -1697,7 +1701,12 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { toggle_workflow_actions() { if (!frappe.model.has_workflow(this.doctype)) return; + + Object.keys(this.workflow_action_items).forEach((key) => { + this.workflow_action_items[key].addClass("disabled"); + }); const checked_items = this.get_checked_items(); + frappe .xcall("frappe.model.workflow.get_common_transition_actions", { docs: checked_items, @@ -1705,6 +1714,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { }) .then((actions) => { Object.keys(this.workflow_action_items).forEach((key) => { + this.workflow_action_items[key].removeClass("disabled"); this.workflow_action_items[key].toggle(actions.includes(key)); }); });