From 64a5ebb1122048820767455c88cf7c2cde152ce8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 20 Feb 2024 20:07:13 +0530 Subject: [PATCH] refactor!: Drop user based workflow action compatibility (#24956) * test: run workflow actions in tests * test: fix workflow action tests * refactor!: Remove compatibility with `user` based workflow action * test: cleanup test cleanups * fix: restore form dict after printing * test: patch printing during workflow tests --- frappe/__init__.py | 4 ++ .../doctype/workflow/test_workflow.py | 37 ++++------------ .../workflow_action/workflow_action.py | 44 ++----------------- 3 files changed, 15 insertions(+), 70 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index ff953cce26..66934778b4 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -10,6 +10,7 @@ be used to build database driven apps. Read the documentation: https://frappeframework.com/docs """ +import copy import functools import gc import importlib @@ -2174,6 +2175,8 @@ def get_print( from frappe.utils.pdf import get_pdf from frappe.website.serve import get_response_content + original_form_dict = copy.deepcopy(local.form_dict) + local.form_dict.doctype = doctype local.form_dict.name = name local.form_dict.format = print_format @@ -2187,6 +2190,7 @@ def get_print( pdf_options["password"] = password html = get_response_content("printview") + local.form_dict = original_form_dict return get_pdf(html, options=pdf_options, output=output) if as_pdf else html diff --git a/frappe/workflow/doctype/workflow/test_workflow.py b/frappe/workflow/doctype/workflow/test_workflow.py index 9df30d25b0..ecd04d5e44 100644 --- a/frappe/workflow/doctype/workflow/test_workflow.py +++ b/frappe/workflow/doctype/workflow/test_workflow.py @@ -1,5 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +from unittest.mock import patch + import frappe from frappe.model.workflow import ( WorkflowTransitionError, @@ -19,10 +21,14 @@ class TestWorkflow(FrappeTestCase): make_test_records("User") def setUp(self): + self.patcher = patch("frappe.attach_print", return_value={}) + self.patcher.start() + frappe.db.delete("Workflow Action") self.workflow = create_todo_workflow() - frappe.set_user("Administrator") def tearDown(self): + frappe.set_user("Administrator") + self.patcher.stop() frappe.delete_doc("Workflow", "Test ToDo") def test_default_condition(self): @@ -82,7 +88,6 @@ class TestWorkflow(FrappeTestCase): self.assertListEqual(actions, ["Review"]) def test_if_workflow_actions_were_processed_using_role(self): - frappe.db.delete("Workflow Action") user = frappe.get_doc("User", "test2@example.com") user.add_roles("Test Approver", "System Manager") frappe.set_user("test2@example.com") @@ -94,35 +99,9 @@ class TestWorkflow(FrappeTestCase): # test if status of workflow actions are updated on approval self.test_approve(doc) user.remove_roles("Test Approver", "System Manager") - workflow_actions = frappe.get_all("Workflow Action", fields=["status"]) - self.assertEqual(len(workflow_actions), 1) - self.assertEqual(workflow_actions[0].status, "Completed") - frappe.set_user("Administrator") - - def test_if_workflow_actions_were_processed_using_user(self): - frappe.db.delete("Workflow Action") - - user = frappe.get_doc("User", "test2@example.com") - user.add_roles("Test Approver", "System Manager") - frappe.set_user("test2@example.com") - - doc = self.test_default_condition() workflow_actions = frappe.get_all("Workflow Action", fields=["*"]) self.assertEqual(len(workflow_actions), 1) - - # test if status of workflow actions are updated on approval - WorkflowAction = DocType("Workflow Action") - WorkflowActionPermittedRole = DocType("Workflow Action Permitted Role") - frappe.qb.update(WorkflowAction).set(WorkflowAction.user, "test2@example.com").run() - frappe.qb.update(WorkflowActionPermittedRole).set(WorkflowActionPermittedRole.role, "").run() - - self.test_approve(doc) - - user.remove_roles("Test Approver", "System Manager") - workflow_actions = frappe.get_all("Workflow Action", fields=["status"]) - self.assertEqual(len(workflow_actions), 1) self.assertEqual(workflow_actions[0].status, "Completed") - frappe.set_user("Administrator") def test_if_workflow_set_on_action(self): self.workflow._update_state_docstatus = True @@ -166,7 +145,7 @@ def create_todo_workflow(): workflow.document_type = "ToDo" workflow.workflow_state_field = "workflow_state" workflow.is_active = 1 - workflow.send_email_alert = 0 + workflow.send_email_alert = 1 workflow.append("states", dict(state="Pending", allow_edit="All")) workflow.append( "states", diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.py b/frappe/workflow/doctype/workflow_action/workflow_action.py index 2a2fef4082..d25a56b9a9 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.py +++ b/frappe/workflow/doctype/workflow_action/workflow_action.py @@ -74,8 +74,7 @@ def get_permission_query_conditions(user): .where(WorkflowActionPermittedRole.role.isin(roles)) ).get_sql() - return f"""(`tabWorkflow Action`.`name` in ({permitted_workflow_actions}) - or `tabWorkflow Action`.`user`={frappe.db.escape(user)}) + return f""" `tabWorkflow Action`.`name` in ({permitted_workflow_actions}) and `tabWorkflow Action`.`status`='Open' """ @@ -118,6 +117,7 @@ def process_workflow_actions(doc, state): doc=doc, transitions=next_possible_transitions, enqueue_after_commit=True, + now=frappe.flags.in_test, ) @@ -212,11 +212,6 @@ def update_completed_workflow_actions(doc, user=None, workflow=None, workflow_st return if workflow_action := get_workflow_action_by_role(doc, allowed_roles): update_completed_workflow_actions_using_role(user, workflow_action) - else: - # backwards compatibility - # for workflow actions saved using user - clear_old_workflow_actions_using_user(doc, user) - update_completed_workflow_actions_using_user(doc, user) def get_allowed_roles(user, workflow, workflow_state): @@ -268,39 +263,6 @@ def update_completed_workflow_actions_using_role(user=None, workflow_action=None ).run() -def clear_old_workflow_actions_using_user(doc, user=None): - user = user if user else frappe.session.user - - if frappe.db.has_column("Workflow Action", "user"): - frappe.db.delete( - "Workflow Action", - { - "reference_name": doc.get("name"), - "reference_doctype": doc.get("doctype"), - "status": "Open", - "user": ("!=", user), - }, - ) - - -def update_completed_workflow_actions_using_user(doc, user=None): - user = user or frappe.session.user - - if frappe.db.has_column("Workflow Action", "user"): - WorkflowAction = DocType("Workflow Action") - ( - frappe.qb.update(WorkflowAction) - .set(WorkflowAction.status, "Completed") - .set(WorkflowAction.completed_by, user) - .where( - (WorkflowAction.reference_name == doc.get("name")) - & (WorkflowAction.reference_doctype == doc.get("doctype")) - & (WorkflowAction.status == "Open") - & (WorkflowAction.user == user) - ) - ).run() - - def get_next_possible_transitions(workflow_name, state, doc=None): transitions = frappe.get_all( "Workflow Transition", @@ -380,7 +342,7 @@ 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 user, data in users_data.items(): # noqa: B007 + for data in users_data.values(): email_args = { "recipients": [data.get("email")], "args": {"actions": list(deduplicate_actions(data.get("possible_actions"))), "message": message},