Refactor Expense Claim (#12883)
* patch for custom workflow
* deleted field approval_status
* replaced approval_status with workflow_state
* updated test cases
* validation to check expense approver
* check if workflow_state_name already exists
* fixes
* modified notifications.py
* removed field exp_approval and modified test cases
diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.js b/erpnext/accounts/doctype/journal_entry/journal_entry.js
index 60c974f..0e03e33 100644
--- a/erpnext/accounts/doctype/journal_entry/journal_entry.js
+++ b/erpnext/accounts/doctype/journal_entry/journal_entry.js
@@ -116,7 +116,6 @@
if(jvd.reference_type==="Expense Claim") {
return {
filters: {
- 'approval_status': 'Approved',
'total_sanctioned_amount': ['>', 0],
'status': ['!=', 'Paid'],
'docstatus': 1
diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.js b/erpnext/accounts/doctype/payment_entry/payment_entry.js
index 496f412..4f77a45 100644
--- a/erpnext/accounts/doctype/payment_entry/payment_entry.js
+++ b/erpnext/accounts/doctype/payment_entry/payment_entry.js
@@ -96,7 +96,7 @@
}
if(child.reference_doctype == "Expense Claim") {
- filters["status"] = "Approved";
+ filters["docstatus"] = 1;
filters["is_paid"] = 0;
}
diff --git a/erpnext/demo/user/hr.py b/erpnext/demo/user/hr.py
index d61aa4e..9b83584 100644
--- a/erpnext/demo/user/hr.py
+++ b/erpnext/demo/user/hr.py
@@ -7,7 +7,7 @@
from erpnext.projects.doctype.timesheet.timesheet import make_salary_slip, make_sales_invoice
from frappe.utils.make_random import get_random
from erpnext.hr.doctype.expense_claim.test_expense_claim import get_payable_account
-from erpnext.hr.doctype.expense_claim.expense_claim import get_expense_approver, make_bank_entry
+from erpnext.hr.doctype.expense_claim.expense_claim import make_bank_entry
from erpnext.hr.doctype.leave_application.leave_application import (get_leave_balance_on,
OverlapError, AttendanceAlreadyMarkedError)
@@ -55,13 +55,11 @@
expense_claim.company = frappe.flags.company
expense_claim.payable_account = get_payable_account(expense_claim.company)
expense_claim.posting_date = frappe.flags.current_date
- expense_claim.exp_approver = filter((lambda x: x[0] != 'Administrator'), get_expense_approver(None, '', None, 0, 20, None))[0][0]
expense_claim.insert()
rand = random.random()
if rand < 0.4:
- expense_claim.approval_status = "Approved"
update_sanctioned_amount(expense_claim)
expense_claim.submit()
@@ -74,10 +72,6 @@
je.flags.ignore_permissions = 1
je.submit()
- elif rand < 0.2:
- expense_claim.approval_status = "Rejected"
- expense_claim.submit()
-
def get_expenses():
expenses = []
expese_types = frappe.db.sql("""select ect.name, eca.default_account from `tabExpense Claim Type` ect,
diff --git a/erpnext/hr/doctype/expense_claim/expense_claim.js b/erpnext/hr/doctype/expense_claim/expense_claim.js
index a99311d..841fd4b 100644
--- a/erpnext/hr/doctype/expense_claim/expense_claim.js
+++ b/erpnext/hr/doctype/expense_claim/expense_claim.js
@@ -38,13 +38,8 @@
cur_frm.add_fetch('employee','employee_name','employee_name');
cur_frm.cscript.onload = function(doc) {
- if(!doc.approval_status)
- cur_frm.set_value("approval_status", "Draft");
-
if (doc.__islocal) {
cur_frm.set_value("posting_date", frappe.datetime.get_today());
- if(doc.amended_from)
- cur_frm.set_value("approval_status", "Draft");
cur_frm.cscript.clear_sanctioned(doc);
}
@@ -53,12 +48,6 @@
query: "erpnext.controllers.queries.employee_query"
};
};
-
- cur_frm.set_query("exp_approver", function() {
- return {
- query: "erpnext.hr.doctype.expense_claim.expense_claim.get_expense_approver"
- };
- });
};
cur_frm.cscript.clear_sanctioned = function(doc) {
@@ -75,13 +64,8 @@
cur_frm.cscript.set_help(doc);
if(!doc.__islocal) {
- cur_frm.toggle_enable("exp_approver", doc.approval_status=="Draft");
- cur_frm.toggle_enable("approval_status", (doc.exp_approver==frappe.session.user && doc.docstatus==0));
- if (doc.docstatus==0 && doc.exp_approver==frappe.session.user && doc.approval_status=="Approved")
- cur_frm.savesubmit();
-
- if (doc.docstatus===1 && doc.approval_status=="Approved") {
+ if (doc.docstatus===1) {
/* eslint-disable */
// no idea how `me` works here
var entry_doctype, entry_reference_doctype, entry_reference_name;
@@ -114,14 +98,6 @@
cur_frm.set_intro("");
if(doc.__islocal && !in_list(frappe.user_roles, "HR User")) {
cur_frm.set_intro(__("Fill the form and save it"));
- } else {
- if(doc.docstatus==0 && doc.approval_status=="Draft") {
- if(frappe.session.user==doc.exp_approver) {
- cur_frm.set_intro(__("You are the Expense Approver for this record. Please Update the 'Status' and Save"));
- } else {
- cur_frm.set_intro(__("Expense Claim is pending approval. Only the Expense Approver can update status."));
- }
- }
}
};
@@ -183,7 +159,7 @@
refresh: function(frm) {
frm.trigger("toggle_fields");
- if(frm.doc.docstatus == 1 && frm.doc.approval_status == 'Approved') {
+ if(frm.doc.docstatus == 1) {
frm.add_custom_button(__('Accounting Ledger'), function() {
frappe.route_options = {
voucher_no: frm.doc.name,
@@ -194,7 +170,7 @@
}, __("View"));
}
- if (frm.doc.docstatus===1 && frm.doc.approval_status=="Approved"
+ if (frm.doc.docstatus===1
&& (cint(frm.doc.total_amount_reimbursed) < cint(frm.doc.total_sanctioned_amount))
&& frappe.model.can_create("Payment Entry")) {
frm.add_custom_button(__('Payment'),
diff --git a/erpnext/hr/doctype/expense_claim/expense_claim.json b/erpnext/hr/doctype/expense_claim/expense_claim.json
index a005ae0..71c6c6e 100644
--- a/erpnext/hr/doctype/expense_claim/expense_claim.json
+++ b/erpnext/hr/doctype/expense_claim/expense_claim.json
@@ -50,104 +50,6 @@
"bold": 0,
"collapsible": 0,
"columns": 0,
- "default": "Draft",
- "depends_on": "eval:!doc.__islocal",
- "fieldname": "approval_status",
- "fieldtype": "Select",
- "hidden": 0,
- "ignore_user_permissions": 0,
- "ignore_xss_filter": 0,
- "in_filter": 0,
- "in_global_search": 0,
- "in_list_view": 0,
- "in_standard_filter": 0,
- "label": "Approval Status",
- "length": 0,
- "no_copy": 1,
- "oldfieldname": "approval_status",
- "oldfieldtype": "Select",
- "options": "Draft\nApproved\nRejected",
- "permlevel": 0,
- "print_hide": 0,
- "print_hide_if_no_value": 0,
- "read_only": 0,
- "remember_last_selected_value": 0,
- "report_hide": 0,
- "reqd": 0,
- "search_index": 1,
- "set_only_once": 0,
- "unique": 0
- },
- {
- "allow_bulk_edit": 0,
- "allow_on_submit": 0,
- "bold": 0,
- "collapsible": 0,
- "columns": 0,
- "description": "A user with \"Expense Approver\" role",
- "fieldname": "exp_approver",
- "fieldtype": "Link",
- "hidden": 0,
- "ignore_user_permissions": 0,
- "ignore_xss_filter": 0,
- "in_filter": 0,
- "in_global_search": 0,
- "in_list_view": 0,
- "in_standard_filter": 0,
- "label": "Approver",
- "length": 0,
- "no_copy": 0,
- "oldfieldname": "exp_approver",
- "oldfieldtype": "Select",
- "options": "User",
- "permlevel": 0,
- "print_hide": 0,
- "print_hide_if_no_value": 0,
- "read_only": 0,
- "remember_last_selected_value": 0,
- "report_hide": 0,
- "reqd": 0,
- "search_index": 0,
- "set_only_once": 0,
- "unique": 0,
- "width": "160px"
- },
- {
- "allow_bulk_edit": 0,
- "allow_on_submit": 0,
- "bold": 0,
- "collapsible": 0,
- "columns": 0,
- "fieldname": "column_break0",
- "fieldtype": "Column Break",
- "hidden": 0,
- "ignore_user_permissions": 0,
- "ignore_xss_filter": 0,
- "in_filter": 0,
- "in_global_search": 0,
- "in_list_view": 0,
- "in_standard_filter": 0,
- "length": 0,
- "no_copy": 0,
- "oldfieldtype": "Column Break",
- "permlevel": 0,
- "print_hide": 0,
- "print_hide_if_no_value": 0,
- "read_only": 0,
- "remember_last_selected_value": 0,
- "report_hide": 0,
- "reqd": 0,
- "search_index": 0,
- "set_only_once": 0,
- "unique": 0,
- "width": "50%"
- },
- {
- "allow_bulk_edit": 0,
- "allow_on_submit": 0,
- "bold": 0,
- "collapsible": 0,
- "columns": 0,
"fieldname": "total_claimed_amount",
"fieldtype": "Currency",
"hidden": 0,
@@ -1058,8 +960,8 @@
"istable": 0,
"max_attachments": 0,
"menu_index": 0,
- "modified": "2017-12-14 17:40:02.959352",
- "modified_by": "nabinhait@gmail.com",
+ "modified": "2018-02-13 12:14:38.236072",
+ "modified_by": "Administrator",
"module": "HR",
"name": "Expense Claim",
"name_case": "Title Case",
@@ -1152,7 +1054,7 @@
"quick_entry": 0,
"read_only": 0,
"read_only_onload": 0,
- "search_fields": "approval_status,employee,employee_name",
+ "search_fields": "employee,employee_name",
"show_name_in_global_search": 1,
"sort_field": "modified",
"sort_order": "DESC",
diff --git a/erpnext/hr/doctype/expense_claim/expense_claim.py b/erpnext/hr/doctype/expense_claim/expense_claim.py
index 9462211..1c87475 100644
--- a/erpnext/hr/doctype/expense_claim/expense_claim.py
+++ b/erpnext/hr/doctype/expense_claim/expense_claim.py
@@ -14,20 +14,16 @@
from frappe.utils.csvutils import getlink
class InvalidExpenseApproverError(frappe.ValidationError): pass
+class ExpenseApproverIdentityError(frappe.ValidationError): pass
class ExpenseClaim(AccountsController):
def onload(self):
self.get("__onload").make_payment_via_journal_entry = frappe.db.get_single_value('Accounts Settings',
'make_payment_via_journal_entry')
- def get_feed(self):
- return _("{0}: From {0} for {1}").format(self.approval_status,
- self.employee_name, self.total_claimed_amount)
-
def validate(self):
self.validate_advances()
self.validate_sanctioned_amount()
- self.validate_expense_approver()
self.calculate_total_amount()
set_employee_name(self)
self.set_expense_account()
@@ -46,12 +42,10 @@
paid_amount = flt(self.total_amount_reimbursed) + flt(self.total_advance_amount)
if self.total_sanctioned_amount > 0 and self.total_sanctioned_amount == paid_amount\
- and self.docstatus == 1 and self.approval_status == 'Approved':
+ and self.docstatus == 1:
self.status = "Paid"
- elif self.total_sanctioned_amount > 0 and self.docstatus == 1 and self.approval_status == 'Approved':
+ elif self.total_sanctioned_amount > 0 and self.docstatus == 1:
self.status = "Unpaid"
- elif self.docstatus == 1 and self.approval_status == 'Rejected':
- self.status = 'Rejected'
def set_payable_account(self):
if not self.payable_account and not self.is_paid:
@@ -62,8 +56,6 @@
self.cost_center = frappe.db.get_value('Company', self.company, 'cost_center')
def on_submit(self):
- if self.approval_status=="Draft":
- frappe.throw(_("""Approval Status must be 'Approved' or 'Rejected'"""))
self.update_task_and_project()
self.make_gl_entries()
@@ -189,17 +181,9 @@
self.total_claimed_amount = 0
self.total_sanctioned_amount = 0
for d in self.get('expenses'):
- if self.approval_status == 'Rejected':
- d.sanctioned_amount = 0.0
-
self.total_claimed_amount += flt(d.claim_amount)
self.total_sanctioned_amount += flt(d.sanctioned_amount)
- def validate_expense_approver(self):
- if self.exp_approver and "Expense Approver" not in frappe.get_roles(self.exp_approver):
- frappe.throw(_("{0} ({1}) must have role 'Expense Approver'")\
- .format(get_fullname(self.exp_approver), self.exp_approver), InvalidExpenseApproverError)
-
def update_task(self):
task = frappe.get_doc("Task", self.task)
task.update_total_expense_claim()
@@ -250,15 +234,6 @@
frappe.db.set_value("Expense Claim", doc.name , "status", doc.status)
@frappe.whitelist()
-def get_expense_approver(doctype, txt, searchfield, start, page_len, filters):
- return frappe.db.sql("""
- select u.name, concat(u.first_name, ' ', u.last_name)
- from tabUser u, `tabHas Role` r
- where u.name = r.parent and r.role = 'Expense Approver'
- and u.enabled = 1 and u.name like %s
- """, ("%" + txt + "%"))
-
-@frappe.whitelist()
def make_bank_entry(dt, dn):
from erpnext.accounts.doctype.journal_entry.journal_entry import get_default_bank_cash_account
diff --git a/erpnext/hr/doctype/expense_claim/expense_claim_list.js b/erpnext/hr/doctype/expense_claim/expense_claim_list.js
index f5a6f4c..0e25e66 100644
--- a/erpnext/hr/doctype/expense_claim/expense_claim_list.js
+++ b/erpnext/hr/doctype/expense_claim/expense_claim_list.js
@@ -1,6 +1,5 @@
frappe.listview_settings['Expense Claim'] = {
- add_fields: ["approval_status", "total_claimed_amount", "docstatus"],
- filters:[["approval_status","!=", "Rejected"]],
+ add_fields: ["total_claimed_amount", "docstatus"],
get_indicator: function(doc) {
if(doc.status == "Paid") {
return [__("Paid"), "green", "status,=,'Paid'"];
diff --git a/erpnext/hr/doctype/expense_claim/test_expense_claim.js b/erpnext/hr/doctype/expense_claim/test_expense_claim.js
index c89eef4..070474e 100644
--- a/erpnext/hr/doctype/expense_claim/test_expense_claim.js
+++ b/erpnext/hr/doctype/expense_claim/test_expense_claim.js
@@ -9,9 +9,8 @@
// Creating Expense Claim
() => frappe.set_route('List','Expense Claim','List'),
() => frappe.timeout(0.3),
- () => frappe.click_button('Make a new Expense Claim'),
+ () => frappe.click_button('New'),
() => {
- cur_frm.set_value('exp_approver','Administrator'),
cur_frm.set_value('is_paid',1),
cur_frm.set_value('expenses',[]),
d = frappe.model.add_child(cur_frm.doc,'Expense Claim Detail','expenses'),
@@ -22,36 +21,23 @@
d.sanctioned_amount=2000,
refresh_field('expenses');
},
- () => frappe.timeout(2),
- () => frappe.db.get_value('Employee', {'employee_name': 'Test Employee 1'}, 'name'),
- (r) => {
- employee_name = r.message.name;
- },
() => frappe.timeout(1),
- () => cur_frm.set_value('employee',employee_name),
- () => cur_frm.set_value('employee_name','Test Employee 1'),
+ () => cur_frm.set_value('employee','Test Employee 1'),
() => cur_frm.set_value('company','For Testing'),
() => cur_frm.set_value('payable_account','Creditors - FT'),
() => cur_frm.set_value('cost_center','Main - FT'),
() => cur_frm.set_value('mode_of_payment','Cash'),
() => cur_frm.save(),
- () => frappe.timeout(1),
- () => cur_frm.set_value('approval_status','Approved'),
- () => frappe.timeout(1),
- () => cur_frm.save(),
- // Submitting the Expense Claim
() => frappe.click_button('Submit'),
() => frappe.click_button('Yes'),
() => frappe.timeout(3),
// Checking if the amount is correctly reimbursed for the employee
() => {
- assert.equal(employee_name,cur_frm.get_field('employee').value,
- 'Expense Claim is created for correct employee');
- assert.equal(1,cur_frm.get_field('is_paid').value,
- 'Expense is paid as required');
- assert.equal(2000,cur_frm.get_field('total_amount_reimbursed').value,
- 'Amount is reimbursed correctly');
+ assert.equal("Test Employee 1",cur_frm.doc.employee, 'Employee name set correctly');
+ assert.equal(1, cur_frm.doc.is_paid, 'Expense is paid as required');
+ assert.equal(2000, cur_frm.doc.total_amount_reimbursed, 'Amount is reimbursed correctly');
+
},
() => done()
]);
diff --git a/erpnext/hr/doctype/expense_claim/test_expense_claim.py b/erpnext/hr/doctype/expense_claim/test_expense_claim.py
index 9b4832a..90b1a8a 100644
--- a/erpnext/hr/doctype/expense_claim/test_expense_claim.py
+++ b/erpnext/hr/doctype/expense_claim/test_expense_claim.py
@@ -81,24 +81,6 @@
self.assertEquals(expected_values[gle.account][1], gle.debit)
self.assertEquals(expected_values[gle.account][2], gle.credit)
- def test_rejected_expense_claim(self):
- payable_account = get_payable_account("Wind Power LLC")
- expense_claim = frappe.get_doc({
- "doctype": "Expense Claim",
- "employee": "_T-Employee-0001",
- "payable_account": payable_account,
- "approval_status": "Rejected",
- "expenses":
- [{ "expense_type": "Travel", "default_account": "Travel Expenses - WP", "claim_amount": 300, "sanctioned_amount": 200 }]
- })
- expense_claim.submit()
-
- self.assertEquals(expense_claim.status, 'Rejected')
- self.assertEquals(expense_claim.total_sanctioned_amount, 0.0)
-
- gl_entry = frappe.get_all('GL Entry', {'voucher_type': 'Expense Claim', 'voucher_no': expense_claim.name})
- self.assertEquals(len(gl_entry), 0)
-
def get_payable_account(company):
return frappe.db.get_value('Company', company, 'default_payable_account')
@@ -107,7 +89,6 @@
"doctype": "Expense Claim",
"employee": "_T-Employee-0001",
"payable_account": payable_account,
- "approval_status": "Approved",
"company": company,
"expenses":
[{ "expense_type": "Travel", "default_account": account, "claim_amount": claim_amount, "sanctioned_amount": sanctioned_amount }]
diff --git a/erpnext/patches.txt b/erpnext/patches.txt
index eabad74..8f9a392 100644
--- a/erpnext/patches.txt
+++ b/erpnext/patches.txt
@@ -494,6 +494,7 @@
erpnext.patches.v10_0.update_sales_order_link_to_purchase_order
erpnext.patches.v10_0.added_extra_gst_custom_field_in_gstr2 #2018-02-13
erpnext.patches.v10_0.item_barcode_childtable_migrate
+erpnext.patches.v10_0.workflow_expense_claim
erpnext.patches.v10_0.set_b2c_limit
erpnext.patches.v10_0.update_translatable_fields
-erpnext.patches.v10_0.rename_offer_letter_to_job_offer
+erpnext.patches.v10_0.rename_offer_letter_to_job_offer
\ No newline at end of file
diff --git a/erpnext/patches/v10_0/workflow_expense_claim.py b/erpnext/patches/v10_0/workflow_expense_claim.py
new file mode 100644
index 0000000..9aa2cee
--- /dev/null
+++ b/erpnext/patches/v10_0/workflow_expense_claim.py
@@ -0,0 +1,65 @@
+# Copyright (c) 2017, Frappe and Contributors
+# License: GNU General Public License v3. See license.txt
+
+from __future__ import unicode_literals
+import frappe
+
+def execute():
+ if frappe.db.a_row_exists("Expense Claim"):
+ frappe.reload_doc("hr", "doctype", "expense_claim")
+ frappe.reload_doc("hr", "doctype", "vehicle_log")
+ frappe.reload_doc("hr", "doctype", "expense_claim_advance")
+ frappe.reload_doc("workflow", "doctype", "workflow")
+
+ states = {'Approved': 'Success', 'Rejected': 'Danger', 'Draft': 'Warning'}
+ for state, style in states.items():
+ if not frappe.db.exists("Workflow State", state):
+ frappe.get_doc({
+ 'doctype': 'Workflow State',
+ 'workflow_state_name': state,
+ 'style': style
+ }).insert(ignore_permissions=True)
+
+ for action in ['Approve', 'Reject']:
+ if not frappe.db.exists("Workflow Action", action):
+ frappe.get_doc({
+ 'doctype': 'Workflow Action',
+ 'workflow_action_name': action
+ }).insert(ignore_permissions=True)
+
+ if not frappe.db.exists("Workflow", "Expense Approval"):
+ frappe.get_doc({
+ 'doctype': 'Workflow',
+ 'workflow_name': 'Expense Approval',
+ 'document_type': 'Expense Claim',
+ 'is_active': 1,
+ 'workflow_state_field': 'workflow_state',
+ 'states': [{
+ "state": 'Draft',
+ "doc_status": 0,
+ "allow_edit": 'Employee'
+ }, {
+ "state": 'Approved',
+ "doc_status": 1,
+ "allow_edit": 'Expense Approver'
+ }, {
+ "state": 'Rejected',
+ "doc_status": 0,
+ "allow_edit": 'Expense Approver'
+ }],
+ 'transitions': [{
+ "state": 'Draft',
+ "action": 'Approve',
+ "next_state": 'Approved',
+ "allowed": 'Expense Approver'
+ },
+ {
+ "state": 'Draft',
+ "action": 'Reject',
+ "next_state": 'Rejected',
+ "allowed": 'Expense Approver'
+ }]
+ }).insert(ignore_permissions=True)
+
+ if frappe.db.has_column("Expense Claim", "status"):
+ frappe.db.sql("""update `tabExpense Claim` set workflow_state = approval_status""")
diff --git a/erpnext/projects/doctype/project/project.py b/erpnext/projects/doctype/project/project.py
index 049c943..d920a09 100644
--- a/erpnext/projects/doctype/project/project.py
+++ b/erpnext/projects/doctype/project/project.py
@@ -170,7 +170,7 @@
from_expense_claim = frappe.db.sql("""select
sum(total_sanctioned_amount) as total_sanctioned_amount
- from `tabExpense Claim` where project = %s and approval_status='Approved'
+ from `tabExpense Claim` where project = %s
and docstatus = 1""",
self.name, as_dict=1)[0]
diff --git a/erpnext/projects/doctype/task/task.py b/erpnext/projects/doctype/task/task.py
index 2d6c973..480926c 100644
--- a/erpnext/projects/doctype/task/task.py
+++ b/erpnext/projects/doctype/task/task.py
@@ -77,7 +77,7 @@
def update_total_expense_claim(self):
self.total_expense_claim = frappe.db.sql("""select sum(total_sanctioned_amount) from `tabExpense Claim`
- where project = %s and task = %s and approval_status = "Approved" and docstatus=1""",(self.project, self.name))[0][0]
+ where project = %s and task = %s and docstatus=1""",(self.project, self.name))[0][0]
def update_time_and_costing(self):
tl = frappe.db.sql("""select min(from_time) as start_date, max(to_time) as end_date,
diff --git a/erpnext/startup/notifications.py b/erpnext/startup/notifications.py
index 690ccf0..3ab2be4 100644
--- a/erpnext/startup/notifications.py
+++ b/erpnext/startup/notifications.py
@@ -30,7 +30,7 @@
},
"Payment Entry": {"docstatus": 0},
"Leave Application": {"docstatus": 0},
- "Expense Claim": {"approval_status": "Draft"},
+ "Expense Claim": {"docstatus": 0},
"Job Applicant": {"status": "Open"},
"Delivery Note": {
"status": ("not in", ("Completed", "Closed")),