diff --git a/frappe/workflow/doctype/workflow/test_workflow.py b/frappe/workflow/doctype/workflow/test_workflow.py index d8d30a5934..9df30d25b0 100644 --- a/frappe/workflow/doctype/workflow/test_workflow.py +++ b/frappe/workflow/doctype/workflow/test_workflow.py @@ -21,35 +21,9 @@ class TestWorkflow(FrappeTestCase): def setUp(self): self.workflow = create_todo_workflow() frappe.set_user("Administrator") - if self._testMethodName == "test_if_workflow_actions_were_processed_using_user": - if not frappe.db.has_column("Workflow Action", "user"): - # mariadb would raise this statement would create an implicit commit - # if we do not commit before alter statement - # nosemgrep - frappe.db.commit() - frappe.db.multisql( - { - "mariadb": "ALTER TABLE `tabWorkflow Action` ADD COLUMN user varchar(140)", - "postgres": 'ALTER TABLE "tabWorkflow Action" ADD COLUMN "user" varchar(140)', - } - ) - frappe.cache.delete_value("table_columns") def tearDown(self): frappe.delete_doc("Workflow", "Test ToDo") - if self._testMethodName == "test_if_workflow_actions_were_processed_using_user": - if frappe.db.has_column("Workflow Action", "user"): - # mariadb would raise this statement would create an implicit commit - # if we do not commit before alter statement - # nosemgrep - frappe.db.commit() - frappe.db.multisql( - { - "mariadb": "ALTER TABLE `tabWorkflow Action` DROP COLUMN user", - "postgres": 'ALTER TABLE "tabWorkflow Action" DROP COLUMN "user"', - } - ) - frappe.cache.delete_value("table_columns") def test_default_condition(self): """test default condition is set""" diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.py b/frappe/workflow/doctype/workflow_action/workflow_action.py index c4180c47ba..1506e9311b 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.py +++ b/frappe/workflow/doctype/workflow_action/workflow_action.py @@ -108,15 +108,17 @@ def process_workflow_actions(doc, state): if not next_possible_transitions: return - user_data_map, roles = get_users_next_action_data(next_possible_transitions, doc) - - if not user_data_map: - return - + roles = {t.allowed for t in next_possible_transitions} create_workflow_actions_for_roles(roles, doc) if send_email_alert(workflow): - enqueue(send_workflow_action_email, queue="short", users_data=list(user_data_map.values()), doc=doc) + enqueue( + send_workflow_action_email, + queue="short", + doc=doc, + transitions=next_possible_transitions, + enqueue_after_commit=True, + ) @frappe.whitelist(allow_guest=True) @@ -321,12 +323,20 @@ def get_next_possible_transitions(workflow_name, state, doc=None): def get_users_next_action_data(transitions, doc): - roles = set() user_data_map = {} + + @frappe.request_cache + def user_has_permission(user: str) -> bool: + from frappe.permissions import has_permission + + return has_permission(doctype=doc, user=user, print_logs=False) + for transition in transitions: - roles.add(transition.allowed) users = get_users_with_role(transition.allowed) - filtered_users = filter_allowed_users(users, doc, transition) + filtered_users = [ + user for user in users if has_approval_access(user, doc, transition) and user_has_permission(user) + ] + for user in filtered_users: if not user_data_map.get(user): user_data_map[user] = frappe._dict( @@ -344,10 +354,12 @@ def get_users_next_action_data(transitions, doc): } ) ) - return user_data_map, roles + return user_data_map def create_workflow_actions_for_roles(roles, doc): + if not roles: + return workflow_action = frappe.get_doc( { "doctype": "Workflow Action", @@ -364,7 +376,8 @@ def create_workflow_actions_for_roles(roles, doc): workflow_action.insert(ignore_permissions=True) -def send_workflow_action_email(users_data, doc): +def send_workflow_action_email(doc, transitions): + users_data = get_users_next_action_data(transitions, doc) common_args = get_common_email_args(doc) message = common_args.pop("message", None) for d in users_data: @@ -451,20 +464,6 @@ def get_doc_workflow_state(doc): return doc.get(workflow_state_field) -def filter_allowed_users(users, doc, transition): - """Filters list of users by checking if user has access to doc and - if the user satisfies 'workflow transision self approval' condition - """ - from frappe.permissions import has_permission - - return [ - user - for user in users - if has_approval_access(user, doc, transition) - and has_permission(doctype=doc, user=user, print_logs=False) - ] - - def get_common_email_args(doc): doctype = doc.get("doctype") docname = doc.get("name")