Merge pull request #25190 from ruchamahabal/approver-perms
feat(HR): share doc with employee approvers if they don't have access
This commit is contained in:
commit
d77ea7c88b
@ -80,6 +80,7 @@ class Employee(NestedSet):
|
||||
self.update_user()
|
||||
self.update_user_permissions()
|
||||
self.reset_employee_emails_cache()
|
||||
self.update_approver_role()
|
||||
|
||||
def update_user_permissions(self):
|
||||
if not self.create_user_permission: return
|
||||
@ -145,6 +146,17 @@ class Employee(NestedSet):
|
||||
|
||||
user.save()
|
||||
|
||||
def update_approver_role(self):
|
||||
if self.leave_approver:
|
||||
user = frappe.get_doc("User", self.leave_approver)
|
||||
user.flags.ignore_permissions = True
|
||||
user.add_roles("Leave Approver")
|
||||
|
||||
if self.expense_approver:
|
||||
user = frappe.get_doc("User", self.expense_approver)
|
||||
user.flags.ignore_permissions = True
|
||||
user.add_roles("Expense Approver")
|
||||
|
||||
def validate_date(self):
|
||||
if self.date_of_birth and getdate(self.date_of_birth) > getdate(today()):
|
||||
throw(_("Date of Birth cannot be greater than today."))
|
||||
@ -503,7 +515,7 @@ def has_user_permission_for_employee(user_name, employee_name):
|
||||
})
|
||||
|
||||
def has_upload_permission(doc, ptype='read', user=None):
|
||||
if not user:
|
||||
if not user:
|
||||
user = frappe.session.user
|
||||
if get_doc_permissions(doc, user=user, ptype=ptype).get(ptype):
|
||||
return True
|
||||
|
@ -6,7 +6,7 @@ import frappe, erpnext
|
||||
from frappe import _
|
||||
from frappe.utils import get_fullname, flt, cstr, get_link_to_form
|
||||
from frappe.model.document import Document
|
||||
from erpnext.hr.utils import set_employee_name
|
||||
from erpnext.hr.utils import set_employee_name, share_doc_with_approver
|
||||
from erpnext.accounts.party import get_party_account
|
||||
from erpnext.accounts.general_ledger import make_gl_entries
|
||||
from erpnext.accounts.doctype.sales_invoice.sales_invoice import get_bank_cash_account
|
||||
@ -53,6 +53,9 @@ class ExpenseClaim(AccountsController):
|
||||
elif self.docstatus == 1 and self.approval_status == 'Rejected':
|
||||
self.status = 'Rejected'
|
||||
|
||||
def on_update(self):
|
||||
share_doc_with_approver(self, self.expense_approver)
|
||||
|
||||
def set_payable_account(self):
|
||||
if not self.payable_account and not self.is_paid:
|
||||
self.payable_account = frappe.get_cached_value('Company', self.company, 'default_expense_claim_payable_account')
|
||||
|
@ -95,12 +95,12 @@ class TestExpenseClaim(unittest.TestCase):
|
||||
def test_rejected_expense_claim(self):
|
||||
payable_account = get_payable_account(company_name)
|
||||
expense_claim = frappe.get_doc({
|
||||
"doctype": "Expense Claim",
|
||||
"employee": "_T-Employee-00001",
|
||||
"payable_account": payable_account,
|
||||
"approval_status": "Rejected",
|
||||
"expenses":
|
||||
[{ "expense_type": "Travel", "default_account": "Travel Expenses - _TC4", "amount": 300, "sanctioned_amount": 200 }]
|
||||
"doctype": "Expense Claim",
|
||||
"employee": "_T-Employee-00001",
|
||||
"payable_account": payable_account,
|
||||
"approval_status": "Rejected",
|
||||
"expenses":
|
||||
[{ "expense_type": "Travel", "default_account": "Travel Expenses - _TC4", "amount": 300, "sanctioned_amount": 200 }]
|
||||
})
|
||||
expense_claim.submit()
|
||||
|
||||
@ -110,6 +110,34 @@ class TestExpenseClaim(unittest.TestCase):
|
||||
gl_entry = frappe.get_all('GL Entry', {'voucher_type': 'Expense Claim', 'voucher_no': expense_claim.name})
|
||||
self.assertEquals(len(gl_entry), 0)
|
||||
|
||||
def test_expense_approver_perms(self):
|
||||
user = "test_approver_perm_emp@example.com"
|
||||
make_employee(user, "_Test Company")
|
||||
|
||||
# check doc shared
|
||||
payable_account = get_payable_account("_Test Company")
|
||||
expense_claim = make_expense_claim(payable_account, 300, 200, "_Test Company", "Travel Expenses - _TC", do_not_submit=True)
|
||||
expense_claim.expense_approver = user
|
||||
expense_claim.save()
|
||||
self.assertTrue(expense_claim.name in frappe.share.get_shared("Expense Claim", user))
|
||||
|
||||
# check shared doc revoked
|
||||
expense_claim.reload()
|
||||
expense_claim.expense_approver = "test@example.com"
|
||||
expense_claim.save()
|
||||
self.assertTrue(expense_claim.name not in frappe.share.get_shared("Expense Claim", user))
|
||||
|
||||
expense_claim.reload()
|
||||
expense_claim.expense_approver = user
|
||||
expense_claim.save()
|
||||
|
||||
frappe.set_user(user)
|
||||
expense_claim.reload()
|
||||
expense_claim.status = "Approved"
|
||||
expense_claim.submit()
|
||||
frappe.set_user("Administrator")
|
||||
|
||||
|
||||
def get_payable_account(company):
|
||||
return frappe.get_cached_value('Company', company, 'default_payable_account')
|
||||
|
||||
@ -133,21 +161,21 @@ def make_expense_claim(payable_account, amount, sanctioned_amount, company, acco
|
||||
|
||||
currency, cost_center = frappe.db.get_value('Company', company, ['default_currency', 'cost_center'])
|
||||
expense_claim = {
|
||||
"doctype": "Expense Claim",
|
||||
"employee": employee,
|
||||
"payable_account": payable_account,
|
||||
"approval_status": "Approved",
|
||||
"company": company,
|
||||
'currency': currency,
|
||||
"expenses": [{
|
||||
"doctype": "Expense Claim",
|
||||
"employee": employee,
|
||||
"payable_account": payable_account,
|
||||
"approval_status": "Approved",
|
||||
"company": company,
|
||||
"currency": currency,
|
||||
"expenses": [{
|
||||
"expense_type": "Travel",
|
||||
"default_account": account,
|
||||
"currency": currency,
|
||||
"amount": amount,
|
||||
"sanctioned_amount": sanctioned_amount,
|
||||
"cost_center": cost_center
|
||||
}]
|
||||
}
|
||||
}]
|
||||
}
|
||||
if taxes:
|
||||
expense_claim.update(taxes)
|
||||
|
||||
|
@ -6,7 +6,7 @@ import frappe
|
||||
from frappe import _
|
||||
from frappe.utils import cint, cstr, date_diff, flt, formatdate, getdate, get_link_to_form, \
|
||||
comma_or, get_fullname, add_days, nowdate, get_datetime_str
|
||||
from erpnext.hr.utils import set_employee_name, get_leave_period
|
||||
from erpnext.hr.utils import set_employee_name, get_leave_period, share_doc_with_approver
|
||||
from erpnext.hr.doctype.leave_block_list.leave_block_list import get_applicable_block_dates
|
||||
from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee
|
||||
from erpnext.buying.doctype.supplier_scorecard.supplier_scorecard import daterange
|
||||
@ -43,6 +43,8 @@ class LeaveApplication(Document):
|
||||
if frappe.db.get_single_value("HR Settings", "send_leave_notification"):
|
||||
self.notify_leave_approver()
|
||||
|
||||
share_doc_with_approver(self, self.leave_approver)
|
||||
|
||||
def on_submit(self):
|
||||
if self.status == "Open":
|
||||
frappe.throw(_("Only Leave Applications with status 'Approved' and 'Rejected' can be submitted"))
|
||||
@ -417,6 +419,7 @@ class LeaveApplication(Document):
|
||||
))
|
||||
create_leave_ledger_entry(self, args, submit)
|
||||
|
||||
|
||||
def get_allocation_expiry(employee, leave_type, to_date, from_date):
|
||||
''' Returns expiry of carry forward allocation in leave ledger entry '''
|
||||
expiry = frappe.get_all("Leave Ledger Entry",
|
||||
|
@ -11,6 +11,7 @@ from frappe.utils import add_days, nowdate, now_datetime, getdate, add_months
|
||||
from erpnext.hr.doctype.leave_type.test_leave_type import create_leave_type
|
||||
from erpnext.hr.doctype.leave_allocation.test_leave_allocation import create_leave_allocation
|
||||
from erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment import create_assignment_for_multiple_employees
|
||||
from erpnext.hr.doctype.employee.test_employee import make_employee
|
||||
|
||||
test_dependencies = ["Leave Allocation", "Leave Block List", "Employee"]
|
||||
|
||||
@ -567,6 +568,48 @@ class TestLeaveApplication(unittest.TestCase):
|
||||
|
||||
self.assertEquals(get_leave_balance_on(employee.name, leave_type.name, add_days(nowdate(), -85), add_days(nowdate(), -84)), 0)
|
||||
|
||||
def test_leave_approver_perms(self):
|
||||
employee = get_employee()
|
||||
user = "test_approver_perm_emp@example.com"
|
||||
make_employee(user, "_Test Company")
|
||||
|
||||
# set approver for employee
|
||||
employee.reload()
|
||||
employee.leave_approver = user
|
||||
employee.save()
|
||||
self.assertTrue("Leave Approver" in frappe.get_roles(user))
|
||||
|
||||
make_allocation_record(employee.name)
|
||||
|
||||
application = self.get_application(_test_records[0])
|
||||
application.from_date = '2018-01-01'
|
||||
application.to_date = '2018-01-03'
|
||||
application.leave_approver = user
|
||||
application.insert()
|
||||
self.assertTrue(application.name in frappe.share.get_shared("Leave Application", user))
|
||||
|
||||
# check shared doc revoked
|
||||
application.reload()
|
||||
application.leave_approver = "test@example.com"
|
||||
application.save()
|
||||
self.assertTrue(application.name not in frappe.share.get_shared("Leave Application", user))
|
||||
|
||||
application.reload()
|
||||
application.leave_approver = user
|
||||
application.save()
|
||||
|
||||
frappe.set_user(user)
|
||||
application.reload()
|
||||
application.status = "Approved"
|
||||
application.submit()
|
||||
|
||||
# unset leave approver
|
||||
frappe.set_user("Administrator")
|
||||
employee.reload()
|
||||
employee.leave_approver = ""
|
||||
employee.save()
|
||||
|
||||
|
||||
def create_carry_forwarded_allocation(employee, leave_type):
|
||||
# initial leave allocation
|
||||
leave_allocation = create_leave_allocation(
|
||||
|
@ -34,8 +34,8 @@ def validate_leave_allocation_against_leave_application(ledger):
|
||||
""", (ledger.employee, ledger.leave_type, ledger.from_date, ledger.to_date))
|
||||
|
||||
if leave_application_records:
|
||||
frappe.throw(_("Leave allocation %s is linked with leave application %s"
|
||||
% (ledger.transaction_name, ', '.join(leave_application_records))))
|
||||
frappe.throw(_("Leave allocation {0} is linked with the Leave Application {1}").format(
|
||||
ledger.transaction_name, ', '.join(leave_application_records)))
|
||||
|
||||
def create_leave_ledger_entry(ref_doc, args, submit=True):
|
||||
ledger = frappe._dict(
|
||||
|
@ -7,6 +7,7 @@ import frappe
|
||||
from frappe import _
|
||||
from frappe.model.document import Document
|
||||
from frappe.utils import formatdate, getdate
|
||||
from erpnext.hr.utils import share_doc_with_approver
|
||||
|
||||
class OverlapError(frappe.ValidationError): pass
|
||||
|
||||
@ -17,6 +18,9 @@ class ShiftRequest(Document):
|
||||
self.validate_approver()
|
||||
self.validate_default_shift()
|
||||
|
||||
def on_update(self):
|
||||
share_doc_with_approver(self, self.approver)
|
||||
|
||||
def on_submit(self):
|
||||
if self.status not in ["Approved", "Rejected"]:
|
||||
frappe.throw(_("Only Shift Request with status 'Approved' and 'Rejected' can be submitted"))
|
||||
@ -29,6 +33,7 @@ class ShiftRequest(Document):
|
||||
if self.to_date:
|
||||
assignment_doc.end_date = self.to_date
|
||||
assignment_doc.shift_request = self.name
|
||||
assignment_doc.flags.ignore_permissions = 1
|
||||
assignment_doc.insert()
|
||||
assignment_doc.submit()
|
||||
|
||||
|
@ -6,6 +6,7 @@ from __future__ import unicode_literals
|
||||
import frappe
|
||||
import unittest
|
||||
from frappe.utils import nowdate, add_days
|
||||
from erpnext.hr.doctype.employee.test_employee import make_employee
|
||||
|
||||
test_dependencies = ["Shift Type"]
|
||||
|
||||
@ -19,19 +20,8 @@ class TestShiftRequest(unittest.TestCase):
|
||||
set_shift_approver(department)
|
||||
approver = frappe.db.sql("""select approver from `tabDepartment Approver` where parent= %s and parentfield = 'shift_request_approver'""", (department))[0][0]
|
||||
|
||||
shift_request = frappe.get_doc({
|
||||
"doctype": "Shift Request",
|
||||
"shift_type": "Day Shift",
|
||||
"company": "_Test Company",
|
||||
"employee": "_T-Employee-00001",
|
||||
"employee_name": "_Test Employee",
|
||||
"from_date": nowdate(),
|
||||
"to_date": add_days(nowdate(), 10),
|
||||
"approver": approver,
|
||||
"status": "Approved"
|
||||
})
|
||||
shift_request.insert()
|
||||
shift_request.submit()
|
||||
shift_request = make_shift_request(approver)
|
||||
|
||||
shift_assignments = frappe.db.sql('''
|
||||
SELECT shift_request, employee
|
||||
FROM `tabShift Assignment`
|
||||
@ -44,8 +34,65 @@ class TestShiftRequest(unittest.TestCase):
|
||||
shift_assignment_doc = frappe.get_doc("Shift Assignment", {"shift_request": d.get('shift_request')})
|
||||
self.assertEqual(shift_assignment_doc.docstatus, 2)
|
||||
|
||||
def test_shift_request_approver_perms(self):
|
||||
employee = frappe.get_doc("Employee", "_T-Employee-00001")
|
||||
user = "test_approver_perm_emp@example.com"
|
||||
make_employee(user, "_Test Company")
|
||||
|
||||
# set approver for employee
|
||||
employee.reload()
|
||||
employee.shift_request_approver = user
|
||||
employee.save()
|
||||
|
||||
shift_request = make_shift_request(user, do_not_submit=True)
|
||||
self.assertTrue(shift_request.name in frappe.share.get_shared("Shift Request", user))
|
||||
|
||||
# check shared doc revoked
|
||||
shift_request.reload()
|
||||
department = frappe.get_value("Employee", "_T-Employee-00001", "department")
|
||||
set_shift_approver(department)
|
||||
department_approver = frappe.db.sql("""select approver from `tabDepartment Approver` where parent= %s and parentfield = 'shift_request_approver'""", (department))[0][0]
|
||||
shift_request.approver = department_approver
|
||||
shift_request.save()
|
||||
self.assertTrue(shift_request.name not in frappe.share.get_shared("Shift Request", user))
|
||||
|
||||
shift_request.reload()
|
||||
shift_request.approver = user
|
||||
shift_request.save()
|
||||
|
||||
frappe.set_user(user)
|
||||
shift_request.reload()
|
||||
shift_request.status = "Approved"
|
||||
shift_request.submit()
|
||||
|
||||
# unset approver
|
||||
frappe.set_user("Administrator")
|
||||
employee.reload()
|
||||
employee.shift_request_approver = ""
|
||||
employee.save()
|
||||
|
||||
|
||||
def set_shift_approver(department):
|
||||
department_doc = frappe.get_doc("Department", department)
|
||||
department_doc.append('shift_request_approver',{'approver': "test1@example.com"})
|
||||
department_doc.save()
|
||||
department_doc.reload()
|
||||
|
||||
def make_shift_request(approver, do_not_submit=0):
|
||||
shift_request = frappe.get_doc({
|
||||
"doctype": "Shift Request",
|
||||
"shift_type": "Day Shift",
|
||||
"company": "_Test Company",
|
||||
"employee": "_T-Employee-00001",
|
||||
"employee_name": "_Test Employee",
|
||||
"from_date": nowdate(),
|
||||
"to_date": add_days(nowdate(), 10),
|
||||
"approver": approver,
|
||||
"status": "Approved"
|
||||
}).insert()
|
||||
|
||||
if do_not_submit:
|
||||
return shift_request
|
||||
|
||||
shift_request.submit()
|
||||
return shift_request
|
@ -504,3 +504,25 @@ def grant_leaves_automatically():
|
||||
lpa = frappe.db.get_all("Leave Policy Assignment", filters={"effective_from": getdate(), "docstatus": 1, "leaves_allocated":0})
|
||||
for assignment in lpa:
|
||||
frappe.get_doc("Leave Policy Assignment", assignment.name).grant_leave_alloc_for_employee()
|
||||
|
||||
def share_doc_with_approver(doc, user):
|
||||
# if approver does not have permissions, share
|
||||
if not frappe.has_permission(doc=doc, ptype="submit", user=user):
|
||||
frappe.share.add(doc.doctype, doc.name, user, submit=1,
|
||||
flags={"ignore_share_permission": True})
|
||||
|
||||
frappe.msgprint(_("Shared with the user {0} with {1} access").format(
|
||||
user, frappe.bold("submit"), alert=True))
|
||||
|
||||
# remove shared doc if approver changes
|
||||
doc_before_save = doc.get_doc_before_save()
|
||||
if doc_before_save:
|
||||
approvers = {
|
||||
"Leave Application": "leave_approver",
|
||||
"Expense Claim": "expense_approver",
|
||||
"Shift Request": "approver"
|
||||
}
|
||||
|
||||
approver = approvers.get(doc.doctype)
|
||||
if doc_before_save.get(approver) != doc.get(approver):
|
||||
frappe.share.remove(doc.doctype, doc.name, doc_before_save.get(approver))
|
||||
|
Loading…
x
Reference in New Issue
Block a user