refactor: Vehicle Expenses Report (#25727)

* refactor: Vehicle Expense Report

* test: Vehicle Expenses Report

* feat: Added Employee filter to report

- fix Vehicle Log form view

* fix: set currency fieldtype for chart data

- added filters for employee and vehicle

* fix: service expenses not getting set
diff --git a/erpnext/hr/doctype/vehicle_log/test_vehicle_log.py b/erpnext/hr/doctype/vehicle_log/test_vehicle_log.py
index cf0048c..ed52c4e 100644
--- a/erpnext/hr/doctype/vehicle_log/test_vehicle_log.py
+++ b/erpnext/hr/doctype/vehicle_log/test_vehicle_log.py
@@ -5,7 +5,7 @@
 
 import frappe
 import unittest
-from frappe.utils import nowdate,flt, cstr,random_string
+from frappe.utils import nowdate, flt, cstr, random_string
 from erpnext.hr.doctype.employee.test_employee import make_employee
 from erpnext.hr.doctype.vehicle_log.vehicle_log import make_expense_claim
 
@@ -18,23 +18,13 @@
 			self.employee_id = make_employee("testdriver@example.com", company="_Test Company")
 
 		self.license_plate = get_vehicle(self.employee_id)
-	
+
 	def tearDown(self):
 		frappe.delete_doc("Vehicle", self.license_plate, force=1)
 		frappe.delete_doc("Employee", self.employee_id, force=1)
 
 	def test_make_vehicle_log_and_syncing_of_odometer_value(self):
-		vehicle_log = frappe.get_doc({
-			"doctype": "Vehicle Log",
-			"license_plate": cstr(self.license_plate),
-			"employee": self.employee_id,
-			"date":frappe.utils.nowdate(),
-			"odometer":5010,
-			"fuel_qty":frappe.utils.flt(50),
-			"price": frappe.utils.flt(500)
-		})
-		vehicle_log.save()
-		vehicle_log.submit()
+		vehicle_log = make_vehicle_log(self.license_plate, self.employee_id)
 
 		#checking value of vehicle odometer value on submit.
 		vehicle = frappe.get_doc("Vehicle", self.license_plate)
@@ -51,19 +41,9 @@
 		self.assertEqual(vehicle.last_odometer, current_odometer - distance_travelled)
 
 		vehicle_log.delete()
-	
+
 	def test_vehicle_log_fuel_expense(self):
-		vehicle_log = frappe.get_doc({
-			"doctype": "Vehicle Log",
-			"license_plate": cstr(self.license_plate),
-			"employee": self.employee_id,
-			"date": frappe.utils.nowdate(),
-			"odometer":5010,
-			"fuel_qty":frappe.utils.flt(50),
-			"price": frappe.utils.flt(500)
-		})
-		vehicle_log.save()
-		vehicle_log.submit()
+		vehicle_log = make_vehicle_log(self.license_plate, self.employee_id)
 
 		expense_claim = make_expense_claim(vehicle_log.name)
 		fuel_expense = expense_claim.expenses[0].amount
@@ -73,6 +53,18 @@
 		frappe.delete_doc("Expense Claim", expense_claim.name)
 		frappe.delete_doc("Vehicle Log", vehicle_log.name)
 
+	def test_vehicle_log_with_service_expenses(self):
+		vehicle_log = make_vehicle_log(self.license_plate, self.employee_id, with_services=True)
+
+		expense_claim = make_expense_claim(vehicle_log.name)
+		expenses = expense_claim.expenses[0].amount
+		self.assertEqual(expenses, 27000)
+
+		vehicle_log.cancel()
+		frappe.delete_doc("Expense Claim", expense_claim.name)
+		frappe.delete_doc("Vehicle Log", vehicle_log.name)
+
+
 def get_vehicle(employee_id):
 	license_plate=random_string(10).upper()
 	vehicle = frappe.get_doc({
@@ -81,15 +73,46 @@
 			"make": "Maruti",
 			"model": "PCM",
 			"employee": employee_id,
-			"last_odometer":5000,
-			"acquisition_date":frappe.utils.nowdate(),
+			"last_odometer": 5000,
+			"acquisition_date": nowdate(),
 			"location": "Mumbai",
 			"chassis_no": "1234ABCD",
 			"uom": "Litre",
-			"vehicle_value":frappe.utils.flt(500000)
+			"vehicle_value": flt(500000)
 		})
 	try:
 		vehicle.insert()
 	except frappe.DuplicateEntryError:
 		pass
-	return license_plate
\ No newline at end of file
+	return license_plate
+
+
+def make_vehicle_log(license_plate, employee_id, with_services=False):
+	vehicle_log = frappe.get_doc({
+		"doctype": "Vehicle Log",
+		"license_plate": cstr(license_plate),
+		"employee": employee_id,
+		"date": nowdate(),
+		"odometer": 5010,
+		"fuel_qty": flt(50),
+		"price": flt(500)
+	})
+
+	if with_services:
+		vehicle_log.append("service_detail", {
+			"service_item": "Oil Change",
+			"type": "Inspection",
+			"frequency": "Mileage",
+			"expense_amount": flt(500)
+		})
+		vehicle_log.append("service_detail", {
+			"service_item": "Wheels",
+			"type": "Change",
+			"frequency": "Half Yearly",
+			"expense_amount": flt(1500)
+		})
+
+	vehicle_log.save()
+	vehicle_log.submit()
+
+	return vehicle_log
\ No newline at end of file
diff --git a/erpnext/hr/doctype/vehicle_log/vehicle_log.json b/erpnext/hr/doctype/vehicle_log/vehicle_log.json
index 619e295..4ea9045 100644
--- a/erpnext/hr/doctype/vehicle_log/vehicle_log.json
+++ b/erpnext/hr/doctype/vehicle_log/vehicle_log.json
@@ -1,4 +1,5 @@
 {
+ "actions": [],
  "autoname": "naming_series:",
  "creation": "2016-09-03 14:14:51.788550",
  "doctype": "DocType",
@@ -10,7 +11,6 @@
   "naming_series",
   "license_plate",
   "employee",
-  "column_break_4",
   "column_break_7",
   "model",
   "make",
@@ -66,10 +66,6 @@
    "reqd": 1
   },
   {
-   "fieldname": "column_break_4",
-   "fieldtype": "Column Break"
-  },
-  {
    "fieldname": "column_break_7",
    "fieldtype": "Column Break"
   },
@@ -142,7 +138,6 @@
   {
    "fieldname": "service_detail",
    "fieldtype": "Table",
-   "label": "Service Detail",
    "options": "Vehicle Service"
   },
   {
@@ -158,7 +153,7 @@
    "fetch_from": "license_plate.last_odometer",
    "fieldname": "last_odometer",
    "fieldtype": "Int",
-   "label": "last Odometer Value ",
+   "label": "Last Odometer Value ",
    "read_only": 1,
    "reqd": 1
   },
@@ -168,7 +163,8 @@
   }
  ],
  "is_submittable": 1,
- "modified": "2020-03-18 16:45:45.060761",
+ "links": [],
+ "modified": "2021-05-17 00:10:21.188352",
  "modified_by": "Administrator",
  "module": "HR",
  "name": "Vehicle Log",
diff --git a/erpnext/hr/report/vehicle_expenses/test_vehicle_expenses.py b/erpnext/hr/report/vehicle_expenses/test_vehicle_expenses.py
new file mode 100644
index 0000000..26e0f26
--- /dev/null
+++ b/erpnext/hr/report/vehicle_expenses/test_vehicle_expenses.py
@@ -0,0 +1,73 @@
+# Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors
+# License: GNU General Public License v3. See license.txt
+
+from __future__ import unicode_literals
+import unittest
+import frappe
+from frappe.utils import getdate
+from erpnext.hr.doctype.employee.test_employee import make_employee
+from erpnext.hr.doctype.vehicle_log.vehicle_log import make_expense_claim
+from erpnext.hr.doctype.vehicle_log.test_vehicle_log import get_vehicle, make_vehicle_log
+from erpnext.hr.report.vehicle_expenses.vehicle_expenses import execute
+from erpnext.accounts.utils import get_fiscal_year
+
+class TestVehicleExpenses(unittest.TestCase):
+	@classmethod
+	def setUpClass(self):
+		frappe.db.sql('delete from `tabVehicle Log`')
+
+		employee_id = frappe.db.sql('''select name from `tabEmployee` where name="testdriver@example.com"''')
+		self.employee_id = employee_id[0][0] if employee_id else None
+		if not self.employee_id:
+			self.employee_id = make_employee('testdriver@example.com', company='_Test Company')
+
+		self.license_plate = get_vehicle(self.employee_id)
+
+	def test_vehicle_expenses_based_on_fiscal_year(self):
+		vehicle_log = make_vehicle_log(self.license_plate, self.employee_id, with_services=True)
+		expense_claim = make_expense_claim(vehicle_log.name)
+
+		# Based on Fiscal Year
+		filters = {
+			'filter_based_on': 'Fiscal Year',
+			'fiscal_year': get_fiscal_year(getdate())[0]
+		}
+
+		report = execute(filters)
+
+		expected_data = [{
+			'vehicle': self.license_plate,
+			'make': 'Maruti',
+			'model': 'PCM',
+			'location': 'Mumbai',
+			'log_name': vehicle_log.name,
+			'odometer': 5010,
+			'date': getdate(),
+			'fuel_qty': 50.0,
+			'fuel_price': 500.0,
+			'fuel_expense': 25000.0,
+			'service_expense': 2000.0,
+			'employee': self.employee_id
+		}]
+
+		self.assertEqual(report[1], expected_data)
+
+		# Based on Date Range
+		fiscal_year = get_fiscal_year(getdate(), as_dict=True)
+		filters = {
+			'filter_based_on': 'Date Range',
+			'from_date': fiscal_year.year_start_date,
+			'to_date': fiscal_year.year_end_date
+		}
+
+		report = execute(filters)
+		self.assertEqual(report[1], expected_data)
+
+		# clean up
+		vehicle_log.cancel()
+		frappe.delete_doc('Expense Claim', expense_claim.name)
+		frappe.delete_doc('Vehicle Log', vehicle_log.name)
+
+	def tearDown(self):
+		frappe.delete_doc('Vehicle', self.license_plate, force=1)
+		frappe.delete_doc('Employee', self.employee_id, force=1)
diff --git a/erpnext/hr/report/vehicle_expenses/vehicle_expenses.js b/erpnext/hr/report/vehicle_expenses/vehicle_expenses.js
index b66bebb..879acd1 100644
--- a/erpnext/hr/report/vehicle_expenses/vehicle_expenses.js
+++ b/erpnext/hr/report/vehicle_expenses/vehicle_expenses.js
@@ -1,31 +1,52 @@
 // Copyright (c) 2016, Frappe Technologies Pvt. Ltd. and contributors
 // For license information, please see license.txt
-frappe.require("assets/erpnext/js/financial_statements.js", function() {
-	frappe.query_reports["Vehicle Expenses"] = {
-		"filters": [
-			{
-				"fieldname": "fiscal_year",
-				"label": __("Fiscal Year"),
-				"fieldtype": "Link",
-				"options": "Fiscal Year",
-				"default": frappe.defaults.get_user_default("fiscal_year"),
-				"reqd": 1,
-				"on_change": function(query_report) {
-					var fiscal_year = query_report.get_values().fiscal_year;
-					if (!fiscal_year) {
-						return;
-					}
-					frappe.model.with_doc("Fiscal Year", fiscal_year, function(r) {
-						var fy = frappe.model.get_doc("Fiscal Year", fiscal_year);
-
-						frappe.query_report.set_filter({
-							from_date: fy.year_start_date,
-							to_date: fy.year_end_date
-						});
-					});
-				}
-			}
-		]
-	}
-});
+frappe.query_reports["Vehicle Expenses"] = {
+	"filters": [
+		{
+			"fieldname": "filter_based_on",
+			"label": __("Filter Based On"),
+			"fieldtype": "Select",
+			"options": ["Fiscal Year", "Date Range"],
+			"default": ["Fiscal Year"],
+			"reqd": 1
+		},
+		{
+			"fieldname": "fiscal_year",
+			"label": __("Fiscal Year"),
+			"fieldtype": "Link",
+			"options": "Fiscal Year",
+			"default": frappe.defaults.get_user_default("fiscal_year"),
+			"depends_on": "eval: doc.filter_based_on == 'Fiscal Year'",
+			"reqd": 1
+		},
+		{
+			"fieldname": "from_date",
+			"label": __("From Date"),
+			"fieldtype": "Date",
+			"reqd": 1,
+			"depends_on": "eval: doc.filter_based_on == 'Date Range'",
+			"default": frappe.datetime.add_months(frappe.datetime.nowdate(), -12)
+		},
+		{
+			"fieldname": "to_date",
+			"label": __("To Date"),
+			"fieldtype": "Date",
+			"reqd": 1,
+			"depends_on": "eval: doc.filter_based_on == 'Date Range'",
+			"default": frappe.datetime.nowdate()
+		},
+		{
+			"fieldname": "vehicle",
+			"label": __("Vehicle"),
+			"fieldtype": "Link",
+			"options": "Vehicle"
+		},
+		{
+			"fieldname": "employee",
+			"label": __("Employee"),
+			"fieldtype": "Link",
+			"options": "Employee"
+		}
+	]
+};
 
diff --git a/erpnext/hr/report/vehicle_expenses/vehicle_expenses.json b/erpnext/hr/report/vehicle_expenses/vehicle_expenses.json
index 2ab0c14..1a3e5a9 100644
--- a/erpnext/hr/report/vehicle_expenses/vehicle_expenses.json
+++ b/erpnext/hr/report/vehicle_expenses/vehicle_expenses.json
@@ -1,20 +1,23 @@
 {
- "add_total_row": 0, 
- "apply_user_permissions": 1, 
- "creation": "2016-09-09 03:33:40.605734", 
- "disabled": 0, 
- "docstatus": 0, 
- "doctype": "Report", 
- "idx": 2, 
- "is_standard": "Yes", 
- "modified": "2017-02-24 19:59:18.641284", 
- "modified_by": "Administrator", 
- "module": "HR", 
- "name": "Vehicle Expenses", 
- "owner": "Administrator", 
- "ref_doctype": "Vehicle", 
- "report_name": "Vehicle Expenses", 
- "report_type": "Script Report", 
+ "add_total_row": 1,
+ "columns": [],
+ "creation": "2016-09-09 03:33:40.605734",
+ "disable_prepared_report": 0,
+ "disabled": 0,
+ "docstatus": 0,
+ "doctype": "Report",
+ "filters": [],
+ "idx": 2,
+ "is_standard": "Yes",
+ "modified": "2021-05-16 22:48:22.767535",
+ "modified_by": "Administrator",
+ "module": "HR",
+ "name": "Vehicle Expenses",
+ "owner": "Administrator",
+ "prepared_report": 0,
+ "ref_doctype": "Vehicle",
+ "report_name": "Vehicle Expenses",
+ "report_type": "Script Report",
  "roles": [
   {
    "role": "Fleet Manager"
diff --git a/erpnext/hr/report/vehicle_expenses/vehicle_expenses.py b/erpnext/hr/report/vehicle_expenses/vehicle_expenses.py
index eab58ff..d847cbb 100644
--- a/erpnext/hr/report/vehicle_expenses/vehicle_expenses.py
+++ b/erpnext/hr/report/vehicle_expenses/vehicle_expenses.py
@@ -5,86 +5,209 @@
 import frappe
 import erpnext
 from frappe import _
-from frappe.utils import flt,cstr
+from frappe.utils import flt
 from erpnext.accounts.report.financial_statements import get_period_list
 
 def execute(filters=None):
-	columns, data, chart = [], [], []
-	if filters.get('fiscal_year'):
-		company = erpnext.get_default_company()
-		period_list = get_period_list(filters.get('fiscal_year'), filters.get('fiscal_year'),
-		'', '', 'Fiscal Year', 'Monthly', company=company)
-		columns=get_columns()
-		data=get_log_data(filters)
-		chart=get_chart_data(data,period_list)
+	filters = frappe._dict(filters or {})
+
+	columns = get_columns()
+	data = get_vehicle_log_data(filters)
+	chart = get_chart_data(data, filters)
+
 	return columns, data, None, chart
 
 def get_columns():
-	columns = [_("License") + ":Link/Vehicle:100", _('Create') + ":data:50",
-		_("Model") + ":data:50", _("Location") + ":data:100",
-		_("Log") + ":Link/Vehicle Log:100", _("Odometer") + ":Int:80",
-		_("Date") + ":Date:100", _("Fuel Qty") + ":Float:80",
-		_("Fuel Price") + ":Float:100",_("Fuel Expense") + ":Float:100",
-		_("Service Expense") + ":Float:100"
+	return [
+		{
+			'fieldname': 'vehicle',
+			'fieldtype': 'Link',
+			'label': _('Vehicle'),
+			'options': 'Vehicle',
+			'width': 150
+		},
+		{
+			'fieldname': 'make',
+			'fieldtype': 'Data',
+			'label': _('Make'),
+			'width': 100
+		},
+		{
+			'fieldname': 'model',
+			'fieldtype': 'Data',
+			'label': _('Model'),
+			'width': 80
+		},
+		{
+			'fieldname': 'location',
+			'fieldtype': 'Data',
+			'label': _('Location'),
+			'width': 100
+		},
+		{
+			'fieldname': 'log_name',
+			'fieldtype': 'Link',
+			'label': _('Vehicle Log'),
+			'options': 'Vehicle Log',
+			'width': 100
+		},
+		{
+			'fieldname': 'odometer',
+			'fieldtype': 'Int',
+			'label': _('Odometer Value'),
+			'width': 120
+		},
+		{
+			'fieldname': 'date',
+			'fieldtype': 'Date',
+			'label': _('Date'),
+			'width': 100
+		},
+		{
+			'fieldname': 'fuel_qty',
+			'fieldtype': 'Float',
+			'label': _('Fuel Qty'),
+			'width': 80
+		},
+		{
+			'fieldname': 'fuel_price',
+			'fieldtype': 'Float',
+			'label': _('Fuel Price'),
+			'width': 100
+		},
+		{
+			'fieldname': 'fuel_expense',
+			'fieldtype': 'Currency',
+			'label': _('Fuel Expense'),
+			'width': 150
+		},
+		{
+			'fieldname': 'service_expense',
+			'fieldtype': 'Currency',
+			'label': _('Service Expense'),
+			'width': 150
+		},
+		{
+			'fieldname': 'employee',
+			'fieldtype': 'Link',
+			'label': _('Employee'),
+			'options': 'Employee',
+			'width': 150
+		}
 	]
+
 	return columns
 
-def get_log_data(filters):
-	fy = frappe.db.get_value('Fiscal Year', filters.get('fiscal_year'), ['year_start_date', 'year_end_date'], as_dict=True)
-	data = frappe.db.sql("""select
-			vhcl.license_plate as "License", vhcl.make as "Make", vhcl.model as "Model",
-			vhcl.location as "Location", log.name as "Log", log.odometer as "Odometer",
-			log.date as "Date", log.fuel_qty as "Fuel Qty", log.price as "Fuel Price",
-			log.fuel_qty * log.price as "Fuel Expense"
-		from
+
+def get_vehicle_log_data(filters):
+	start_date, end_date = get_period_dates(filters)
+	conditions, values = get_conditions(filters)
+
+	data = frappe.db.sql("""
+		SELECT
+			vhcl.license_plate as vehicle, vhcl.make, vhcl.model,
+			vhcl.location, log.name as log_name, log.odometer,
+			log.date, log.employee, log.fuel_qty,
+			log.price as fuel_price,
+			log.fuel_qty * log.price as fuel_expense
+		FROM
 			`tabVehicle` vhcl,`tabVehicle Log` log
-		where
-			vhcl.license_plate = log.license_plate and log.docstatus = 1 and date between %s and %s
-		order by date""" ,(fy.year_start_date, fy.year_end_date), as_dict=1)
-	dl=list(data)
-	for row in dl:
-		row["Service Expense"]= get_service_expense(row["Log"])
-	return dl
+		WHERE
+			vhcl.license_plate = log.license_plate
+			and log.docstatus = 1
+			and date between %(start_date)s and %(end_date)s
+			{0}
+		ORDER BY date""".format(conditions), values, as_dict=1)
+
+	for row in data:
+		row['service_expense'] = get_service_expense(row.log_name)
+
+	return data
+
+
+def get_conditions(filters):
+	conditions = ''
+
+	start_date, end_date = get_period_dates(filters)
+	values = {
+		'start_date': start_date,
+		'end_date': end_date
+	}
+
+	if filters.employee:
+		conditions += ' and log.employee = %(employee)s'
+		values['employee'] = filters.employee
+
+	if filters.vehicle:
+		conditions += ' and vhcl.license_plate = %(vehicle)s'
+		values['vehicle'] = filters.vehicle
+
+	return conditions, values
+
+
+def get_period_dates(filters):
+	if filters.filter_based_on == 'Fiscal Year' and filters.fiscal_year:
+		fy = frappe.db.get_value('Fiscal Year', filters.fiscal_year,
+			['year_start_date', 'year_end_date'], as_dict=True)
+		return fy.year_start_date, fy.year_end_date
+	else:
+		return filters.from_date, filters.to_date
+
 
 def get_service_expense(logname):
-	expense_amount = frappe.db.sql("""select sum(expense_amount)
-		from `tabVehicle Log` log,`tabVehicle Service` ser
-		where ser.parent=log.name and log.name=%s""",logname)
-	return flt(expense_amount[0][0]) if expense_amount else 0
+	expense_amount = frappe.db.sql("""
+		SELECT sum(expense_amount)
+		FROM
+			`tabVehicle Log` log, `tabVehicle Service` service
+		WHERE
+			service.parent=log.name and log.name=%s
+	""", logname)
 
-def get_chart_data(data,period_list):
-	fuel_exp_data,service_exp_data,fueldata,servicedata = [],[],[],[]
-	service_exp_data = []
-	fueldata = []
+	return flt(expense_amount[0][0]) if expense_amount else 0.0
+
+
+def get_chart_data(data, filters):
+	period_list = get_period_list(filters.fiscal_year, filters.fiscal_year,
+		filters.from_date, filters.to_date, filters.filter_based_on, 'Monthly')
+
+	fuel_data, service_data = [], []
+
 	for period in period_list:
-		total_fuel_exp=0
-		total_ser_exp=0
-		for row in data:
-			if row["Date"] <= period.to_date and row["Date"] >= period.from_date:
-				total_fuel_exp+=flt(row["Fuel Expense"])
-				total_ser_exp+=flt(row["Service Expense"])
-		fueldata.append([period.key,total_fuel_exp])
-		servicedata.append([period.key,total_ser_exp])
+		total_fuel_exp = 0
+		total_service_exp = 0
 
-	labels = [period.key for period in period_list]
-	fuel_exp_data= [row[1] for row in fueldata]
-	service_exp_data= [row[1] for row in servicedata]
+		for row in data:
+			if row.date <= period.to_date and row.date >= period.from_date:
+				total_fuel_exp += flt(row.fuel_expense)
+				total_service_exp += flt(row.service_expense)
+
+		fuel_data.append([period.key, total_fuel_exp])
+		service_data.append([period.key, total_service_exp])
+
+	labels = [period.label for period in period_list]
+	fuel_exp_data= [row[1] for row in fuel_data]
+	service_exp_data= [row[1] for row in service_data]
+
 	datasets = []
 	if fuel_exp_data:
 		datasets.append({
-			'name': 'Fuel Expenses',
+			'name': _('Fuel Expenses'),
 			'values': fuel_exp_data
 		})
+
 	if service_exp_data:
 		datasets.append({
-			'name': 'Service Expenses',
+			'name': _('Service Expenses'),
 			'values': service_exp_data
 		})
+
 	chart = {
-		"data": {
+		'data': {
 			'labels': labels,
 			'datasets': datasets
-		}
+		},
+		'type': 'line',
+		'fieldtype': 'Currency'
 	}
-	chart["type"] = "line"
+
 	return chart