refactor: deprecate role replication and move to custom button
This commit is contained in:
Thomas Antony
parent
9e7fc6730a
commit
43c47b347d
7 changed files with 100 additions and 231 deletions
|
|
@ -17,13 +17,78 @@ frappe.ui.form.on("Role", {
|
|||
|
||||
frm.set_df_property("is_custom", "read_only", frappe.session.user !== "Administrator");
|
||||
|
||||
frm.add_custom_button("Role Permissions Manager", function () {
|
||||
frappe.route_options = { role: frm.doc.name };
|
||||
frappe.set_route("permission-manager");
|
||||
});
|
||||
frm.add_custom_button("Show Users", function () {
|
||||
frappe.route_options = { role: frm.doc.name };
|
||||
frappe.set_route("List", "User", "Report");
|
||||
});
|
||||
frm.add_custom_button(
|
||||
__("Role Permissions Manager"),
|
||||
function () {
|
||||
frappe.route_options = { role: frm.doc.name };
|
||||
frappe.set_route("permission-manager");
|
||||
},
|
||||
__("View")
|
||||
);
|
||||
|
||||
frm.add_custom_button(
|
||||
__("Show Users"),
|
||||
function () {
|
||||
frappe.route_options = { role: frm.doc.name };
|
||||
frappe.set_route("List", "User", "Report");
|
||||
},
|
||||
__("View")
|
||||
);
|
||||
|
||||
if (frappe.user.has_role("System Manager")) {
|
||||
frm.add_custom_button(
|
||||
__("Replicate Role"),
|
||||
function () {
|
||||
replicate_role(frm);
|
||||
},
|
||||
__("Action")
|
||||
);
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
function replicate_role(frm) {
|
||||
const dialog = new frappe.ui.Dialog({
|
||||
title: __("Replicate Role"),
|
||||
fields: [
|
||||
{
|
||||
label: __("New Role Name"),
|
||||
fieldname: "new_role_name",
|
||||
fieldtype: "Data",
|
||||
default: frm.doc.name,
|
||||
reqd: 1,
|
||||
},
|
||||
],
|
||||
freeze: true,
|
||||
freeze_message: __("Replicating Role..."),
|
||||
primary_action_label: __("Replicate"),
|
||||
primary_action: function (values) {
|
||||
dialog.hide();
|
||||
frappe.call({
|
||||
method: "replicate_role",
|
||||
doc: frm.doc,
|
||||
args: {
|
||||
cur_role: frm.doc.name,
|
||||
new_role: values.new_role_name,
|
||||
},
|
||||
callback: function (r) {
|
||||
if (r.message) {
|
||||
frappe.set_route("Form", "Role", r.message);
|
||||
frappe.show_alert({
|
||||
message: __("New role created successfully."),
|
||||
indicator: "green",
|
||||
});
|
||||
} else if (r.exc) {
|
||||
JSON.parse(r.exc).forEach((err) => {
|
||||
frappe.show_alert({
|
||||
message: __(err),
|
||||
indicator: "red",
|
||||
});
|
||||
});
|
||||
}
|
||||
},
|
||||
});
|
||||
},
|
||||
});
|
||||
dialog.show();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
# License: MIT. See LICENSE
|
||||
|
||||
import frappe
|
||||
from frappe.core.page.permission_manager.permission_manager import get_permissions
|
||||
from frappe.model.document import Document
|
||||
from frappe.website.path_resolver import validate_path
|
||||
from frappe.website.router import clear_routing_cache
|
||||
|
|
@ -86,6 +87,32 @@ class Role(Document):
|
|||
if user_type != user.user_type:
|
||||
user.save()
|
||||
|
||||
@frappe.whitelist()
|
||||
def replicate_role(self, cur_role: str, new_role: str) -> str:
|
||||
frappe.only_for("System Manager")
|
||||
|
||||
if frappe.db.get_value("Role", new_role, "name"):
|
||||
return frappe.errprint(f"Role {new_role} already exist.")
|
||||
|
||||
new_role = frappe.get_doc({"doctype": "Role", "role_name": new_role}).insert().name
|
||||
|
||||
perms = get_permissions(role=cur_role)
|
||||
for perm in perms:
|
||||
perm.update(
|
||||
{
|
||||
"name": None,
|
||||
"creation": None,
|
||||
"modified": None,
|
||||
"modified_by": None,
|
||||
"owner": None,
|
||||
"linked_doctypes": None,
|
||||
"role": new_role,
|
||||
}
|
||||
)
|
||||
frappe.get_doc({"doctype": "Custom DocPerm", **perm}).insert()
|
||||
|
||||
return new_role
|
||||
|
||||
|
||||
def get_info_based_on_role(role, field="email", ignore_permissions=False):
|
||||
"""Get information of all users that have been assigned this role"""
|
||||
|
|
|
|||
|
|
@ -1,18 +0,0 @@
|
|||
// Copyright (c) 2024, Frappe Technologies and contributors
|
||||
// For license information, please see license.txt
|
||||
|
||||
frappe.ui.form.on("Role Replication", {
|
||||
refresh(frm) {
|
||||
frm.disable_save();
|
||||
frm.page.set_primary_action(__("Replicate"), ($btn) => {
|
||||
$btn.text(__("Replicating..."));
|
||||
frappe.run_serially([
|
||||
() => frappe.dom.freeze("Replicating..."),
|
||||
() => frm.call("replicate_role"),
|
||||
() => frappe.dom.unfreeze(),
|
||||
() => frappe.msgprint(__("Replication completed.")),
|
||||
() => $btn.text(__("Replicate")),
|
||||
]);
|
||||
});
|
||||
},
|
||||
});
|
||||
|
|
@ -1,52 +0,0 @@
|
|||
{
|
||||
"actions": [],
|
||||
"creation": "2024-06-24 18:25:23.163914",
|
||||
"doctype": "DocType",
|
||||
"engine": "InnoDB",
|
||||
"field_order": [
|
||||
"existing_role",
|
||||
"column_break_ydyj",
|
||||
"new_role"
|
||||
],
|
||||
"fields": [
|
||||
{
|
||||
"fieldname": "existing_role",
|
||||
"fieldtype": "Link",
|
||||
"label": "Existing Role",
|
||||
"options": "Role"
|
||||
},
|
||||
{
|
||||
"fieldname": "column_break_ydyj",
|
||||
"fieldtype": "Column Break"
|
||||
},
|
||||
{
|
||||
"description": "Input existing role name if you would like to extend it with access of another role.",
|
||||
"fieldname": "new_role",
|
||||
"fieldtype": "Data",
|
||||
"label": "New Role"
|
||||
}
|
||||
],
|
||||
"index_web_pages_for_search": 1,
|
||||
"issingle": 1,
|
||||
"links": [],
|
||||
"modified": "2024-06-24 19:26:54.279801",
|
||||
"modified_by": "Administrator",
|
||||
"module": "Core",
|
||||
"name": "Role Replication",
|
||||
"owner": "Administrator",
|
||||
"permissions": [
|
||||
{
|
||||
"create": 1,
|
||||
"delete": 1,
|
||||
"email": 1,
|
||||
"print": 1,
|
||||
"read": 1,
|
||||
"role": "System Manager",
|
||||
"share": 1,
|
||||
"write": 1
|
||||
}
|
||||
],
|
||||
"sort_field": "creation",
|
||||
"sort_order": "DESC",
|
||||
"states": []
|
||||
}
|
||||
|
|
@ -1,55 +0,0 @@
|
|||
# Copyright (c) 2024, Frappe Technologies and contributors
|
||||
# For license information, please see license.txt
|
||||
|
||||
import frappe
|
||||
from frappe.core.page.permission_manager.permission_manager import get_permissions
|
||||
from frappe.model.document import Document
|
||||
from frappe.permissions import setup_custom_perms
|
||||
|
||||
|
||||
class RoleReplication(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
|
||||
|
||||
existing_role: DF.Link | None
|
||||
new_role: DF.Data | None
|
||||
# end: auto-generated types
|
||||
|
||||
@frappe.whitelist()
|
||||
def replicate_role(self):
|
||||
frappe.only_for("System Manager")
|
||||
|
||||
new_role = frappe.db.get_value("Role", self.new_role, "name")
|
||||
if not new_role:
|
||||
new_role = frappe.get_doc({"doctype": "Role", "role_name": self.new_role}).insert().name
|
||||
|
||||
perms = get_permissions(role=self.existing_role)
|
||||
|
||||
doctypes_with_custom_perms_setup = set()
|
||||
for perm in perms:
|
||||
doctype = perm.get("parent")
|
||||
if doctype and doctype not in doctypes_with_custom_perms_setup:
|
||||
# if no Custom DocPerm exists for the doctype, move standard permissions to Custom DocPerm
|
||||
# before creating first Custom DocPerm for the new role
|
||||
setup_custom_perms(doctype)
|
||||
doctypes_with_custom_perms_setup.add(doctype)
|
||||
|
||||
# Create Custom DocPerm for the new role
|
||||
frappe.get_doc(
|
||||
{
|
||||
"doctype": "Custom DocPerm",
|
||||
**perm,
|
||||
"name": None,
|
||||
"creation": None,
|
||||
"modified": None,
|
||||
"modified_by": None,
|
||||
"owner": None,
|
||||
"linked_doctypes": None,
|
||||
"role": new_role,
|
||||
}
|
||||
).insert()
|
||||
|
|
@ -1,98 +0,0 @@
|
|||
# Copyright (c) 2024, Frappe Technologies and Contributors
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
from frappe.permissions import get_all_perms
|
||||
from frappe.tests import IntegrationTestCase
|
||||
|
||||
|
||||
class TestRoleReplication(IntegrationTestCase):
|
||||
def setUp(self):
|
||||
# Create a test role with permissions
|
||||
self.test_role_name = "_Test Role For Replication"
|
||||
self.new_role_name = "_Test Replicated Role"
|
||||
|
||||
# Clean up any existing test roles and permissions
|
||||
self._cleanup_test_data()
|
||||
|
||||
# Create the test role
|
||||
self.test_role = frappe.get_doc({"doctype": "Role", "role_name": self.test_role_name}).insert()
|
||||
|
||||
# Add a DocPerm permission (simulating standard permission)
|
||||
# We use a doctype that doesn't have Custom DocPerm to simulate the bug scenario
|
||||
self.test_doctype = "User"
|
||||
|
||||
# First ensure no Custom DocPerm exists for this doctype
|
||||
frappe.db.delete("Custom DocPerm", {"parent": self.test_doctype})
|
||||
|
||||
# Add DocPerm for the test role
|
||||
self.test_perm = frappe.get_doc(
|
||||
{
|
||||
"doctype": "DocPerm",
|
||||
"parent": self.test_doctype,
|
||||
"parenttype": "DocType",
|
||||
"parentfield": "permissions",
|
||||
"role": self.test_role_name,
|
||||
"permlevel": 0,
|
||||
"read": 1,
|
||||
"write": 1,
|
||||
"create": 0,
|
||||
}
|
||||
).insert()
|
||||
|
||||
def _cleanup_test_data(self):
|
||||
"""Clean up test roles and permissions."""
|
||||
for role_name in [self.test_role_name, self.new_role_name]:
|
||||
frappe.db.delete("Custom DocPerm", {"role": role_name})
|
||||
frappe.db.delete("DocPerm", {"role": role_name})
|
||||
if frappe.db.exists("Role", role_name):
|
||||
frappe.delete_doc("Role", role_name, force=True)
|
||||
|
||||
def tearDown(self):
|
||||
self._cleanup_test_data()
|
||||
|
||||
def test_replicate_role_preserves_original_permissions(self):
|
||||
"""
|
||||
Test that replicating a role does not erase the original role's permissions.
|
||||
This is a regression test for https://github.com/frappe/frappe/issues/34605
|
||||
"""
|
||||
# Get original permissions count before replication using get_all_perms
|
||||
# (this is what the Role Permissions Manager UI uses)
|
||||
original_perms_before = get_all_perms(self.test_role_name)
|
||||
self.assertTrue(
|
||||
len(original_perms_before) > 0, "Test role should have permissions before replication"
|
||||
)
|
||||
|
||||
# Perform role replication
|
||||
role_replication = frappe.get_doc(
|
||||
{
|
||||
"doctype": "Role Replication",
|
||||
"existing_role": self.test_role_name,
|
||||
"new_role": self.new_role_name,
|
||||
}
|
||||
)
|
||||
role_replication.replicate_role()
|
||||
|
||||
# Verify new role was created
|
||||
self.assertTrue(frappe.db.exists("Role", self.new_role_name), "New role should be created")
|
||||
|
||||
# Verify new role has permissions
|
||||
new_role_perms = get_all_perms(self.new_role_name)
|
||||
self.assertTrue(len(new_role_perms) > 0, "New role should have permissions after replication")
|
||||
|
||||
# Verify original role still has its permissions visible via get_all_perms
|
||||
original_perms_after = get_all_perms(self.test_role_name)
|
||||
self.assertEqual(
|
||||
len(original_perms_before),
|
||||
len(original_perms_after),
|
||||
"Original role should retain all its permissions after replication",
|
||||
)
|
||||
|
||||
# Verify the original role now has Custom DocPerm entries
|
||||
original_custom_perms = frappe.get_all(
|
||||
"Custom DocPerm", filters={"role": self.test_role_name}, fields=["parent", "read", "write"]
|
||||
)
|
||||
self.assertTrue(
|
||||
len(original_custom_perms) > 0,
|
||||
"Original role should have Custom DocPerm entries after replication to preserve visibility",
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue