From 50dd9f31c0fa141649ef07c71778409c04cdb737 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 3 Nov 2025 17:48:57 +0530 Subject: [PATCH] refactor: create same perm type for multiple doctypes --- .../permission_inspector.js | 13 ++-- .../permission_inspector.py | 7 +-- .../permission_type/permission_type.json | 29 ++++----- .../permission_type/permission_type.py | 63 ++++++++++--------- .../permission_type/test_permission_type.py | 14 ++++- .../permission_type_doctype/__init__.py | 0 .../permission_type_doctype.json | 35 +++++++++++ .../permission_type_doctype.py | 23 +++++++ .../permission_manager/permission_manager.js | 16 ++--- .../permission_manager/permission_manager.py | 8 +-- .../impersonate/impersonate.json | 10 ++- frappe/permissions.py | 4 +- 12 files changed, 137 insertions(+), 85 deletions(-) create mode 100644 frappe/core/doctype/permission_type_doctype/__init__.py create mode 100644 frappe/core/doctype/permission_type_doctype/permission_type_doctype.json create mode 100644 frappe/core/doctype/permission_type_doctype/permission_type_doctype.py diff --git a/frappe/core/doctype/permission_inspector/permission_inspector.js b/frappe/core/doctype/permission_inspector/permission_inspector.js index a9f5eb921e..81ad4e5b6a 100644 --- a/frappe/core/doctype/permission_inspector/permission_inspector.js +++ b/frappe/core/doctype/permission_inspector/permission_inspector.js @@ -23,18 +23,13 @@ frappe.ui.form.on("Permission Inspector", { } }, add_custom_perm_types(frm) { - const custom_perm_types = frm.doc.__onload.custom_perm_types - if (!custom_perm_types?.length) return if (!frm.doc.ref_doctype) return - const standard_options = frm.meta.fields.find(f => f.fieldname === "permission_type").options; + const doctype_ptype_map = frm.doc.__onload.doctype_ptype_map + if (!Object.keys(doctype_ptype_map).length) return - const custom_options = ( - custom_perm_types - .filter(pt => pt.applicable_doctype != frm.doc.ref_doctype) - .map(pt => pt.label || pt.name) - .join("\n") - ); + const standard_options = frm.meta.fields.find(f => f.fieldname === "permission_type").options; + const custom_options = doctype_ptype_map[frm.doc.ref_doctype].join("\n"); frm.set_df_property("permission_type", "options", `${standard_options}\n${custom_options}`); } diff --git a/frappe/core/doctype/permission_inspector/permission_inspector.py b/frappe/core/doctype/permission_inspector/permission_inspector.py index 082d79a73f..099ae2166e 100644 --- a/frappe/core/doctype/permission_inspector/permission_inspector.py +++ b/frappe/core/doctype/permission_inspector/permission_inspector.py @@ -38,10 +38,9 @@ class PermissionInspector(Document): # end: auto-generated types def onload(self): - self.set_onload("custom_perm_types", frappe.get_all( - "Permission Type", - fields=["name", "label", "applicable_for"], - )) + from frappe.core.doctype.permission_type.permission_type import get_doctype_ptype_map + + self.set_onload("doctype_ptype_map", get_doctype_ptype_map()) @frappe.whitelist() def debug(self): diff --git a/frappe/core/doctype/permission_type/permission_type.json b/frappe/core/doctype/permission_type/permission_type.json index 0d933e8212..ce9cf3b7ae 100644 --- a/frappe/core/doctype/permission_type/permission_type.json +++ b/frappe/core/doctype/permission_type/permission_type.json @@ -5,33 +5,30 @@ "doctype": "DocType", "engine": "InnoDB", "field_order": [ - "applicable_for", - "column_break_fyaj", - "label" + "module", + "doc_types" ], "fields": [ { - "fieldname": "applicable_for", - "fieldtype": "Link", - "in_list_view": 1, - "label": "Applicable for", - "options": "DocType", + "fieldname": "doc_types", + "fieldtype": "Table MultiSelect", + "label": "Applies To", + "options": "Permission Type DocType", "reqd": 1 }, { - "fieldname": "label", - "fieldtype": "Data", - "label": "Label" - }, - { - "fieldname": "column_break_fyaj", - "fieldtype": "Column Break" + "fieldname": "module", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Module", + "options": "Module Def", + "reqd": 1 } ], "grid_page_length": 50, "index_web_pages_for_search": 1, "links": [], - "modified": "2025-10-30 18:52:03.750433", + "modified": "2025-11-03 15:53:10.367326", "modified_by": "Administrator", "module": "Core", "name": "Permission Type", diff --git a/frappe/core/doctype/permission_type/permission_type.py b/frappe/core/doctype/permission_type/permission_type.py index e862ade980..76f8258164 100644 --- a/frappe/core/doctype/permission_type/permission_type.py +++ b/frappe/core/doctype/permission_type/permission_type.py @@ -21,10 +21,11 @@ class PermissionType(Document): from typing import TYPE_CHECKING if TYPE_CHECKING: + from frappe.core.doctype.permission_type_doctype.permission_type_doctype import PermissionTypeDocType from frappe.types import DF - applicable_for: DF.Link - label: DF.Data | None + doc_types: DF.TableMultiSelect[PermissionTypeDocType] + module: DF.Link # end: auto-generated types def validate(self): @@ -36,11 +37,7 @@ class PermissionType(Document): ) def can_write(self): - return ( - frappe.conf.developer_mode - or frappe.flags.in_migrate - or frappe.flags.in_install - ) + return frappe.conf.developer_mode or frappe.flags.in_migrate or frappe.flags.in_install def on_update(self): if not self.can_write(): @@ -48,22 +45,21 @@ class PermissionType(Document): from frappe.modules.export_file import export_to_files - module = get_doctype_module(self.applicable_for) - export_to_files(record_list=[["Permission Type", self.name]], record_module=module) + export_to_files(record_list=[["Permission Type", self.name]], record_module=self.module) - for doctype in CUSTOM_FIELD_TARGET: - self.create_custom_field(doctype) + for target in CUSTOM_FIELD_TARGET: + self.create_custom_field(target) - def create_custom_field(self, doctype): + def create_custom_field(self, target): from frappe.custom.doctype.custom_field.custom_field import create_custom_field - if not self.custom_field_exists(doctype): - depends_on = f"eval:doc.parent == '{self.applicable_for}'" - if doctype == "DocShare": - depends_on = f"eval:doc.share_doctype == '{self.applicable_for}'" + if not self.custom_field_exists(target): + field = "share_doctype" if target == "DocShare" else "parent" + doc_types = [dt.doc_type for dt in self.doc_types if dt.doc_type] + depends_on = f"eval:{frappe.as_json(doc_types)}.includes(doc.{field})" create_custom_field( - doctype, + target, { "fieldname": self.name, "label": self.name.replace("_", " ").title(), @@ -77,34 +73,39 @@ class PermissionType(Document): if not self.can_write(): frappe.throw(_("Deletion of this document is only permitted in developer mode.")) - for doctype in CUSTOM_FIELD_TARGET: - self.delete_custom_field(doctype) + for target in CUSTOM_FIELD_TARGET: + self.delete_custom_field(target) - module = get_doctype_module(self.applicable_for) - delete_folder(module, "Permission Type", self.name) + delete_folder(self.module, "Permission Type", self.name) - def delete_custom_field(self, doctype): - if name := self.custom_field_exists(doctype): + def delete_custom_field(self, target): + if name := self.custom_field_exists(target): frappe.delete_doc("Custom Field", name) - def custom_field_exists(self, doctype): + def custom_field_exists(self, target): return frappe.db.exists( "Custom Field", { "fieldname": self.name, - "dt": doctype, + "dt": target, }, ) @site_cache -def get_custom_ptype_map(): - ptypes = frappe.get_all( +def get_doctype_ptype_map(): + ptypes = frappe.qb.get_query( "Permission Type", - fields=["name", "label", "applicable_for"], + fields=[ + "name", + {"doc_types": ["doc_type"]}, + ], order_by="name", ) - custom_ptype_map = defaultdict(list) + ptypes = ptypes.run(as_dict=True) + + doctype_ptype_map = defaultdict(list) for pt in ptypes: - custom_ptype_map[pt["applicable_for"]].append(pt["label"] or pt["name"]) - return dict(custom_ptype_map) + for dt in pt.doc_types: + doctype_ptype_map[dt.doc_type].append(pt.name) + return dict(doctype_ptype_map) diff --git a/frappe/core/doctype/permission_type/test_permission_type.py b/frappe/core/doctype/permission_type/test_permission_type.py index a3a2f26a26..700e70605a 100644 --- a/frappe/core/doctype/permission_type/test_permission_type.py +++ b/frappe/core/doctype/permission_type/test_permission_type.py @@ -4,7 +4,6 @@ # import frappe from frappe.tests import IntegrationTestCase - # On IntegrationTestCase, the doctype test records and all # link-field test record dependencies are recursively loaded # Use these module variables to add/remove to/from that list @@ -12,11 +11,20 @@ EXTRA_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] IGNORE_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] - class IntegrationTestPermissionType(IntegrationTestCase): """ Integration tests for PermissionType. Use this class for testing interactions between multiple components. """ - pass + def test_permission_type_creation_deletion(self): ... + + def test_permission_type_creation_reserved_name(self): ... + + def test_role_permission_with_custom_permission_type(self): ... + + def test_share_permission_with_custom_permission_type(self): ... + + def test_role_permission_mapping_with_custom_permission_type(self): ... + + def test_export_import_of_permission_type(self): ... diff --git a/frappe/core/doctype/permission_type_doctype/__init__.py b/frappe/core/doctype/permission_type_doctype/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/core/doctype/permission_type_doctype/permission_type_doctype.json b/frappe/core/doctype/permission_type_doctype/permission_type_doctype.json new file mode 100644 index 0000000000..ff5daf4394 --- /dev/null +++ b/frappe/core/doctype/permission_type_doctype/permission_type_doctype.json @@ -0,0 +1,35 @@ +{ + "actions": [], + "allow_rename": 1, + "creation": "2025-11-03 15:51:52.422122", + "doctype": "DocType", + "engine": "InnoDB", + "field_order": [ + "doc_type" + ], + "fields": [ + { + "fieldname": "doc_type", + "fieldtype": "Link", + "in_list_view": 1, + "label": "DocType", + "options": "DocType", + "reqd": 1 + } + ], + "grid_page_length": 50, + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2025-11-03 15:52:16.161580", + "modified_by": "Administrator", + "module": "Core", + "name": "Permission Type DocType", + "owner": "Administrator", + "permissions": [], + "row_format": "Dynamic", + "rows_threshold_for_grid_search": 20, + "sort_field": "creation", + "sort_order": "DESC", + "states": [] +} diff --git a/frappe/core/doctype/permission_type_doctype/permission_type_doctype.py b/frappe/core/doctype/permission_type_doctype/permission_type_doctype.py new file mode 100644 index 0000000000..3b3922c24c --- /dev/null +++ b/frappe/core/doctype/permission_type_doctype/permission_type_doctype.py @@ -0,0 +1,23 @@ +# Copyright (c) 2025, Frappe Technologies and contributors +# For license information, please see license.txt + +# import frappe +from frappe.model.document import Document + + +class PermissionTypeDocType(Document): + # begin: auto-generated types + # This code is auto-generated. Do not modify anything in this block. + + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from frappe.types import DF + + doc_type: DF.Link + parent: DF.Data + parentfield: DF.Data + parenttype: DF.Data + # end: auto-generated types + + pass diff --git a/frappe/core/page/permission_manager/permission_manager.js b/frappe/core/page/permission_manager/permission_manager.js index 6c3f88ba87..129f1aea4a 100644 --- a/frappe/core/page/permission_manager/permission_manager.js +++ b/frappe/core/page/permission_manager/permission_manager.js @@ -132,19 +132,14 @@ frappe.PermissionEngine = class PermissionEngine { .append(`
${__("Standard Permissions")}:

`); let $wrapper = $("

").appendTo($d); data.message.forEach((d) => { - let rights = this.rights + let custom_rights = this.options.doctype_ptype_map[doctype] || []; + d.rights = this.rights .filter((r) => d[r]) + .concat(custom_rights) .map((r) => { return __(toTitle(frappe.unscrub(r))); }); - this.options.custom_rights.forEach((r) => { - if (r.applicable_for !== doctype || !d[r.name]) return; - rights.push(__(toTitle(frappe.unscrub(r.label || r.name)))); - }); - - d.rights = rights.join(", "); - $wrapper.append(`
\
${__(d.role)}, ${__("Level")} ${d.permlevel || 0}
\
${d.rights}
\ @@ -270,9 +265,8 @@ frappe.PermissionEngine = class PermissionEngine { } }); - this.options.custom_rights.forEach((r) => { - if (r.applicable_for !== d.parent) return; - this.add_check(perm_container, d, r.name); + this.options.doctype_ptype_map[d.parent]?.forEach((r) => { + this.add_check(perm_container, d, r); }); // buttons diff --git a/frappe/core/page/permission_manager/permission_manager.py b/frappe/core/page/permission_manager/permission_manager.py index 1d4eb343b8..b27edb47f6 100644 --- a/frappe/core/page/permission_manager/permission_manager.py +++ b/frappe/core/page/permission_manager/permission_manager.py @@ -9,6 +9,7 @@ from frappe.core.doctype.doctype.doctype import ( clear_permissions_cache, validate_permissions_for_doctype, ) +from frappe.core.doctype.permission_type.permission_type import get_doctype_ptype_map from frappe.exceptions import DoesNotExistError from frappe.modules.import_file import get_file_path, read_doc_from_file from frappe.permissions import ( @@ -57,18 +58,13 @@ def get_roles_and_doctypes(): fields=["name"], ) - custom_rights = frappe.get_all( - "Permission Type", - fields=["name", "label", "applicable_for"], - ) - doctypes_list = [{"label": _(d.get("name")), "value": d.get("name")} for d in doctypes] roles_list = [{"label": _(d.get("name")), "value": d.get("name")} for d in roles] return { "doctypes": sorted(doctypes_list, key=lambda d: d["label"].casefold()), "roles": sorted(roles_list, key=lambda d: d["label"].casefold()), - "custom_rights": custom_rights, + "doctype_ptype_map": get_doctype_ptype_map(), } diff --git a/frappe/core/permission_type/impersonate/impersonate.json b/frappe/core/permission_type/impersonate/impersonate.json index 8c773aacc1..0b8f74493e 100644 --- a/frappe/core/permission_type/impersonate/impersonate.json +++ b/frappe/core/permission_type/impersonate/impersonate.json @@ -1,12 +1,16 @@ { - "applicable_for": "User", "creation": "2025-10-30 19:17:39.833189", + "doc_types": [ + { + "doc_type": "User" + } + ], "docstatus": 0, "doctype": "Permission Type", "idx": 0, - "label": "impersonate", - "modified": "2025-10-30 19:17:39.833189", + "modified": "2025-11-03 16:58:56.819971", "modified_by": "Administrator", + "module": "Core", "name": "impersonate", "owner": "Administrator" } diff --git a/frappe/permissions.py b/frappe/permissions.py index d6a1272856..82fdf7a4c1 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -6,7 +6,7 @@ import functools import frappe import frappe.share from frappe import _, msgprint -from frappe.core.doctype.permission_type.permission_type import get_custom_ptype_map +from frappe.core.doctype.permission_type.permission_type import get_doctype_ptype_map from frappe.query_builder import DocType from frappe.utils import cint, cstr @@ -168,7 +168,7 @@ def has_permission( def false_if_not_shared(): std_rights = ["read", "write", "share", "submit", "email", "print"] - custom_rights = get_custom_ptype_map().get(doctype, []) + custom_rights = get_doctype_ptype_map().get(doctype, []) if ptype not in std_rights + custom_rights: debug and _debug_log(f"Permission type {ptype} can not be shared")