From 07e5786e1bcf45d2874e0de03538a14b34793261 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 10 Dec 2018 19:10:18 +0530 Subject: [PATCH] Fix user permission checks --- erpnext/accounts/party.py | 8 +-- .../leave_application/leave_application.js | 4 +- ...ip_user_permission_check_for_department.py | 64 ++++++++++++++----- erpnext/public/js/utils.js | 2 +- ...rehouse_wise_item_balance_age_and_value.py | 5 +- 5 files changed, 56 insertions(+), 27 deletions(-) diff --git a/erpnext/accounts/party.py b/erpnext/accounts/party.py index f19aaf833b..e3235f006f 100644 --- a/erpnext/accounts/party.py +++ b/erpnext/accounts/party.py @@ -72,7 +72,7 @@ def _get_party_details(party=None, account=None, party_type="Customer", company= return out -def set_address_details(out, party, party_type, doctype=None, company=None, party_address=None, shipping_address=None): +def set_address_details(out, party, party_type, doctype=None, company=None, party_address=None, shipping_address=None): billing_address_field = "customer_address" if party_type == "Lead" \ else party_type.lower() + "_address" out[billing_address_field] = party_address or get_default_address(party_type, party.name) @@ -151,10 +151,8 @@ def get_default_price_list(party): def set_price_list(out, party, party_type, given_price_list): # price list - price_list = filter(None, get_user_permissions() - .get("Price List", {}) - .get("docs", [])) - price_list = list(price_list) + price_list = [d.get('doc') for d in get_user_permissions().get('Price List', []) \ + if d.get('doc')] if price_list: price_list = price_list[0] diff --git a/erpnext/hr/doctype/leave_application/leave_application.js b/erpnext/hr/doctype/leave_application/leave_application.js index a77dd32c49..5bce348489 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.js +++ b/erpnext/hr/doctype/leave_application/leave_application.js @@ -14,7 +14,7 @@ frappe.ui.form.on("Leave Application", { doctype: frm.doc.doctype } }; - }); + }); frm.set_query("employee", erpnext.queries.employee); }, @@ -83,7 +83,7 @@ frappe.ui.form.on("Leave Application", { if (!frm.doc.employee && frappe.defaults.get_user_permissions()) { const perm = frappe.defaults.get_user_permissions(); if (perm && perm['Employee']) { - frm.set_value('employee', perm['Employee']["docs"][0]) + frm.set_value('employee', perm['Employee'].map(perm_doc => perm_doc.doc)[0]); } } }, diff --git a/erpnext/patches/v11_0/skip_user_permission_check_for_department.py b/erpnext/patches/v11_0/skip_user_permission_check_for_department.py index 123eed5aff..46ad6f300b 100644 --- a/erpnext/patches/v11_0/skip_user_permission_check_for_department.py +++ b/erpnext/patches/v11_0/skip_user_permission_check_for_department.py @@ -1,28 +1,60 @@ import frappe +from frappe.desk.form.linked_with import get_linked_doctypes # Skips user permission check for doctypes where department link field was recently added # https://github.com/frappe/erpnext/pull/14121 def execute(): - user_permissions = frappe.get_all("User Permission", - filters=[['allow', '=', 'Department']], - fields=['name', 'skip_for_doctype']) + doctypes_to_skip = [] + for doctype in ['Appraisal', 'Leave Allocation', 'Expense Claim', 'Instructor', 'Salary Slip', + 'Attendance', 'Training Feedback', 'Training Result Employee', + 'Leave Application', 'Employee Advance', 'Activity Cost', 'Training Event Employee', + 'Timesheet', 'Sales Person', 'Payroll Employee Detail']: + if frappe.db.exists('Custom Field', { 'dt': doctype, 'fieldname': 'department'}): continue + doctypes_to_skip.append(doctype) - doctypes_to_skip = [] + frappe.reload_doctype('User Permission') - for doctype in ['Appraisal', 'Leave Allocation', 'Expense Claim', 'Instructor', 'Salary Slip', - 'Attendance', 'Training Feedback', 'Training Result Employee', - 'Leave Application', 'Employee Advance', 'Activity Cost', 'Training Event Employee', - 'Timesheet', 'Sales Person', 'Payroll Employee Detail']: - if frappe.db.exists('Custom Field', { 'dt': doctype, 'fieldname': 'department'}): continue - doctypes_to_skip.append(doctype) + user_permissions = frappe.get_all("User Permission", + filters=[['allow', '=', 'Department'], ['applicable_for', 'in', [None] + doctypes_to_skip]], + fields=['name', 'applicable_for']) - for perm in user_permissions: - skip_for_doctype = perm.get('skip_for_doctype') + user_permissions_to_delete = [] + new_user_permissions_list = [] - skip_for_doctype = skip_for_doctype.split('\n') + doctypes_to_skip - skip_for_doctype = set(skip_for_doctype) # to remove duplicates - skip_for_doctype = '\n'.join(skip_for_doctype) # convert back to string + for user_permission in user_permissions: + if user_permission.applicable_for: + # simply delete user permission record since it needs to be skipped. + user_permissions_to_delete.append(user_permission.name) + else: + # if applicable_for is `None` it means that user permission is applicable for every doctype + # to avoid this we need to create other user permission records and only skip the listed doctypes in this patch + linked_doctypes = get_linked_doctypes(user_permission.allow, True).keys() + applicable_for_doctypes = list(set(linked_doctypes) - set(doctypes_to_skip)) - frappe.set_value('User Permission', perm.name, 'skip_for_doctype', skip_for_doctype) + user_permissions_to_delete.append(user_permission.name) + for doctype in applicable_for_doctypes: + if doctype: + # Maintain sequence (name, user, allow, for_value, applicable_for, apply_to_all_doctypes) + new_user_permissions_list.append(( + frappe.generate_hash("", 10), + user_permission.user, + user_permission.allow, + user_permission.for_value, + doctype, + 0 + )) + + if new_user_permissions_list: + frappe.db.sql(''' + INSERT INTO `tabUser Permission` + (`name`, `user`, `allow`, `for_value`, `applicable_for`, `apply_to_all_doctypes`) + VALUES {}'''.format(', '.join(['%s'] * len(new_user_permissions_list))), + tuple(new_user_permissions_list) + ) + + if user_permissions_to_delete: + frappe.db.sql('DELETE FROM `tabUser Permission` WHERE `name` IN ({})'.format( + ','.join(['%s'] * len(user_permissions_to_delete)) + ), tuple(user_permissions_to_delete)) \ No newline at end of file diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js index baee68e6be..3c14cb449b 100644 --- a/erpnext/public/js/utils.js +++ b/erpnext/public/js/utils.js @@ -217,7 +217,7 @@ $.extend(erpnext.utils, { let unscrub_option = frappe.model.unscrub(option); let user_permission = frappe.defaults.get_user_permissions(); if(user_permission && user_permission[unscrub_option]) { - return user_permission[unscrub_option]["docs"]; + return user_permission[unscrub_option].map(perm => perm.doc); } else { return $.map(locals[`:${unscrub_option}`], function(c) { return c.name; }).sort(); } diff --git a/erpnext/stock/report/warehouse_wise_item_balance_age_and_value/warehouse_wise_item_balance_age_and_value.py b/erpnext/stock/report/warehouse_wise_item_balance_age_and_value/warehouse_wise_item_balance_age_and_value.py index 676cf54d8e..7628368051 100644 --- a/erpnext/stock/report/warehouse_wise_item_balance_age_and_value/warehouse_wise_item_balance_age_and_value.py +++ b/erpnext/stock/report/warehouse_wise_item_balance_age_and_value/warehouse_wise_item_balance_age_and_value.py @@ -87,9 +87,8 @@ def validate_filters(filters): def get_warehouse_list(filters): from frappe.defaults import get_user_permissions condition = '' - user_permitted_warehouse = filter(None, get_user_permissions() - .get("Warehouse", {}) - .get("docs", [])) + user_permitted_warehouse = [d.get('doc') for d in get_user_permissions().get('Warehouse', []) \ + if d.get('doc')] value = () if user_permitted_warehouse: condition = "and name in %s"