Merge branch 'develop' into payroll-entry-fixes
diff --git a/erpnext/payroll/doctype/payroll_employee_detail/payroll_employee_detail.json b/erpnext/payroll/doctype/payroll_employee_detail/payroll_employee_detail.json
index 8a55224..09c7eb9 100644
--- a/erpnext/payroll/doctype/payroll_employee_detail/payroll_employee_detail.json
+++ b/erpnext/payroll/doctype/payroll_employee_detail/payroll_employee_detail.json
@@ -17,8 +17,7 @@
    "fieldtype": "Link",
    "in_list_view": 1,
    "label": "Employee",
-   "options": "Employee",
-   "read_only": 1
+   "options": "Employee"
   },
   {
    "fetch_from": "employee.employee_name",
@@ -52,7 +51,7 @@
  ],
  "istable": 1,
  "links": [],
- "modified": "2020-09-30 12:40:07.999878",
+ "modified": "2020-12-17 15:43:29.542977",
  "modified_by": "Administrator",
  "module": "Payroll",
  "name": "Payroll Employee Detail",
diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js
index cb48abb..2288a27 100644
--- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js
+++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js
@@ -10,15 +10,22 @@
 		}
 		frm.toggle_reqd(['payroll_frequency'], !frm.doc.salary_slip_based_on_timesheet);
 
-		frm.set_query("department", function() {
+		frm.events.department_filters(frm);
+		frm.events.payroll_payable_account_filters(frm);
+	},
+
+	department_filters: function (frm) {
+		frm.set_query("department", function () {
 			return {
 				"filters": {
 					"company": frm.doc.company,
 				}
 			};
 		});
+	},
 
-		frm.set_query("payroll_payable_account", function() {
+	payroll_payable_account_filters: function (frm) {
+		frm.set_query("payroll_payable_account", function () {
 			return {
 				filters: {
 					"company": frm.doc.company,
@@ -29,12 +36,12 @@
 		});
 	},
 
-	refresh: function(frm) {
+	refresh: function (frm) {
 		if (frm.doc.docstatus == 0) {
-			if(!frm.is_new()) {
+			if (!frm.is_new()) {
 				frm.page.clear_primary_action();
 				frm.add_custom_button(__("Get Employees"),
-					function() {
+					function () {
 						frm.events.get_employee_details(frm);
 					}
 				).toggleClass('btn-primary', !(frm.doc.employees || []).length);
@@ -42,7 +49,7 @@
 			if ((frm.doc.employees || []).length) {
 				frm.page.clear_primary_action();
 				frm.page.set_primary_action(__('Create Salary Slips'), () => {
-					frm.save('Submit').then(()=>{
+					frm.save('Submit').then(() => {
 						frm.page.clear_primary_action();
 						frm.refresh();
 						frm.events.refresh(frm);
@@ -61,48 +68,48 @@
 			doc: frm.doc,
 			method: 'fill_employee_details',
 		}).then(r => {
-			if (r.docs && r.docs[0].employees){
+			if (r.docs && r.docs[0].employees) {
 				frm.employees = r.docs[0].employees;
 				frm.dirty();
 				frm.save();
 				frm.refresh();
-				if(r.docs[0].validate_attendance){
+				if (r.docs[0].validate_attendance) {
 					render_employee_attendance(frm, r.message);
 				}
 			}
-		})
+		});
 	},
 
-	create_salary_slips: function(frm) {
+	create_salary_slips: function (frm) {
 		frm.call({
 			doc: frm.doc,
 			method: "create_salary_slips",
-			callback: function(r) {
+			callback: function () {
 				frm.refresh();
 				frm.toolbar.refresh();
 			}
-		})
+		});
 	},
 
-	add_context_buttons: function(frm) {
-		if(frm.doc.salary_slips_submitted || (frm.doc.__onload && frm.doc.__onload.submitted_ss)) {
+	add_context_buttons: function (frm) {
+		if (frm.doc.salary_slips_submitted || (frm.doc.__onload && frm.doc.__onload.submitted_ss)) {
 			frm.events.add_bank_entry_button(frm);
-		} else if(frm.doc.salary_slips_created) {
-			frm.add_custom_button(__("Submit Salary Slip"), function() {
+		} else if (frm.doc.salary_slips_created) {
+			frm.add_custom_button(__("Submit Salary Slip"), function () {
 				submit_salary_slip(frm);
 			}).addClass("btn-primary");
 		}
 	},
 
-	add_bank_entry_button: function(frm) {
+	add_bank_entry_button: function (frm) {
 		frappe.call({
 			method: 'erpnext.payroll.doctype.payroll_entry.payroll_entry.payroll_entry_has_bank_entries',
 			args: {
 				'name': frm.doc.name
 			},
-			callback: function(r) {
+			callback: function (r) {
 				if (r.message && !r.message.submitted) {
-					frm.add_custom_button("Make Bank Entry", function() {
+					frm.add_custom_button("Make Bank Entry", function () {
 						make_bank_entry(frm);
 					}).addClass("btn-primary");
 				}
@@ -141,8 +148,37 @@
 	},
 
 	payroll_frequency: function (frm) {
-		frm.trigger("set_start_end_dates");
-		frm.events.clear_employee_table(frm);
+		frm.trigger("set_start_end_dates").then( ()=> {
+			frm.events.clear_employee_table(frm);
+			frm.events.get_employee_with_salary_slip_and_set_query(frm);
+		});
+	},
+
+	employee_filters: function (frm, emp_list) {
+		frm.set_query('employee', 'employees', () => {
+			return {
+				filters: {
+					name: ["not in", emp_list]
+				}
+			};
+		});
+	},
+
+	get_employee_with_salary_slip_and_set_query: function (frm) {
+		frappe.db.get_list('Salary Slip', {
+			filters: {
+				start_date: frm.doc.start_date,
+				end_date: frm.doc.end_date,
+				docstatus: 1,
+			},
+			fields: ['employee']
+		}).then((emp) => {
+			var emp_list = [];
+			emp.forEach((employee_data) => {
+				emp_list.push(Object.values(employee_data)[0]);
+			});
+			frm.events.employee_filters(frm, emp_list);
+		});
 	},
 
 	company: function (frm) {
@@ -164,17 +200,17 @@
 						from_currency: frm.doc.currency,
 						to_currency: company_currency,
 					},
-					callback: function(r) {
+					callback: function (r) {
 						frm.set_value("exchange_rate", flt(r.message));
 						frm.set_df_property('exchange_rate', 'hidden', 0);
-						frm.set_df_property("exchange_rate", "description", "1 " + frm.doc.currency
-							+ " = [?] " + company_currency);
+						frm.set_df_property("exchange_rate", "description", "1 " + frm.doc.currency +
+							" = [?] " + company_currency);
 					}
 				});
 			} else {
 				frm.set_value("exchange_rate", 1.0);
 				frm.set_df_property('exchange_rate', 'hidden', 1);
-				frm.set_df_property("exchange_rate", "description", "" );
+				frm.set_df_property("exchange_rate", "description", "");
 			}
 		}
 	},
@@ -192,9 +228,9 @@
 	},
 
 	start_date: function (frm) {
-		if(!in_progress && frm.doc.start_date){
+		if (!in_progress && frm.doc.start_date) {
 			frm.trigger("set_end_date");
-		}else{
+		} else {
 			// reset flag
 			in_progress = false;
 		}
@@ -228,7 +264,7 @@
 		}
 	},
 
-	set_end_date: function(frm){
+	set_end_date: function (frm) {
 		frappe.call({
 			method: 'erpnext.payroll.doctype.payroll_entry.payroll_entry.get_end_date',
 			args: {
@@ -243,19 +279,19 @@
 		});
 	},
 
-	validate_attendance: function(frm){
-		if(frm.doc.validate_attendance && frm.doc.employees){
+	validate_attendance: function (frm) {
+		if (frm.doc.validate_attendance && frm.doc.employees) {
 			frappe.call({
 				method: 'validate_employee_attendance',
 				args: {},
-				callback: function(r) {
+				callback: function (r) {
 					render_employee_attendance(frm, r.message);
 				},
 				doc: frm.doc,
 				freeze: true,
 				freeze_message: __('Validating Employee Attendance...')
 			});
-		}else{
+		} else {
 			frm.fields_dict.attendance_detail_html.html("");
 		}
 	},
@@ -270,18 +306,20 @@
 
 const submit_salary_slip = function (frm) {
 	frappe.confirm(__('This will submit Salary Slips and create accrual Journal Entry. Do you want to proceed?'),
-		function() {
+		function () {
 			frappe.call({
 				method: 'submit_salary_slips',
 				args: {},
-				callback: function() {frm.events.refresh(frm);},
+				callback: function () {
+					frm.events.refresh(frm);
+				},
 				doc: frm.doc,
 				freeze: true,
 				freeze_message: __('Submitting Salary Slips and creating Journal Entry...')
 			});
 		},
-		function() {
-			if(frappe.dom.freeze_count) {
+		function () {
+			if (frappe.dom.freeze_count) {
 				frappe.dom.unfreeze();
 				frm.events.refresh(frm);
 			}
@@ -295,9 +333,11 @@
 		return frappe.call({
 			doc: cur_frm.doc,
 			method: "make_payment_entry",
-			callback: function() {
+			callback: function () {
 				frappe.set_route(
-					'List', 'Journal Entry', {"Journal Entry Account.reference_name": frm.doc.name}
+					'List', 'Journal Entry', {
+						"Journal Entry Account.reference_name": frm.doc.name
+					}
 				);
 			},
 			freeze: true,
@@ -309,11 +349,19 @@
 	}
 };
 
-
-let render_employee_attendance = function(frm, data) {
+let render_employee_attendance = function (frm, data) {
 	frm.fields_dict.attendance_detail_html.html(
 		frappe.render_template('employees_to_mark_attendance', {
 			data: data
 		})
 	);
-}
+};
+
+frappe.ui.form.on('Payroll Employee Detail', {
+	employee: function(frm) {
+		frm.events.clear_employee_table(frm);
+		if (!frm.doc.payroll_frequency) {
+			frappe.throw(__("Please set a Payroll Frequency"));
+		}
+	}
+});
diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.json b/erpnext/payroll/doctype/payroll_entry/payroll_entry.json
index 7a48dd1..0444134 100644
--- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.json
+++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.json
@@ -129,8 +129,7 @@
    "fieldname": "employees",
    "fieldtype": "Table",
    "label": "Employee Details",
-   "options": "Payroll Employee Detail",
-   "read_only": 1
+   "options": "Payroll Employee Detail"
   },
   {
    "fieldname": "section_break_13",
@@ -290,7 +289,7 @@
  "icon": "fa fa-cog",
  "is_submittable": 1,
  "links": [],
- "modified": "2020-10-23 13:00:33.753228",
+ "modified": "2020-12-17 15:13:17.766210",
  "modified_by": "Administrator",
  "module": "Payroll",
  "name": "Payroll Entry",
diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py
index 8c2d974..a25a6e7 100644
--- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py
+++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py
@@ -6,7 +6,7 @@
 import frappe, erpnext
 from frappe.model.document import Document
 from dateutil.relativedelta import relativedelta
-from frappe.utils import cint, flt, nowdate, add_days, getdate, fmt_money, add_to_date, DATE_FORMAT, date_diff
+from frappe.utils import cint, flt, add_days, getdate, add_to_date, DATE_FORMAT, date_diff, comma_and
 from frappe import _
 from erpnext.accounts.utils import get_fiscal_year
 from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee
@@ -19,16 +19,26 @@
 		# check if salary slips were manually submitted
 		entries = frappe.db.count("Salary Slip", {'payroll_entry': self.name, 'docstatus': 1}, ['name'])
 		if cint(entries) == len(self.employees):
-    			self.set_onload("submitted_ss", True)
+				self.set_onload("submitted_ss", True)
 
 	def on_submit(self):
 		self.create_salary_slips()
 
 	def before_submit(self):
+		self.validate_employee_details()
 		if self.validate_attendance:
 			if self.validate_employee_attendance():
 				frappe.throw(_("Cannot Submit, Employees left to mark attendance"))
 
+	def validate_employee_details(self):
+		emp_with_sal_slip = []
+		for employee_details in self.employees:
+			if frappe.db.exists("Salary Slip", {"employee": employee_details.employee, "start_date": self.start_date, "end_date": self.end_date, "docstatus": 1}):
+				emp_with_sal_slip.append(employee_details.employee)
+
+		if len(emp_with_sal_slip):
+			frappe.throw(_("Salary Slip already exists for {0} ").format(comma_and(emp_with_sal_slip)))
+
 	def on_cancel(self):
 		frappe.delete_doc("Salary Slip", frappe.db.sql_list("""select name from `tabSalary Slip`
 			where payroll_entry=%s """, (self.name)))
@@ -71,8 +81,17 @@
 					and t2.docstatus = 1
 			%s order by t2.from_date desc
 			""" % cond, {"sal_struct": tuple(sal_struct), "from_date": self.end_date, "payroll_payable_account": self.payroll_payable_account}, as_dict=True)
+
+			emp_list = self.remove_payrolled_employees(emp_list)
 			return emp_list
 
+	def remove_payrolled_employees(self, emp_list):
+		for employee_details in emp_list:
+			if frappe.db.exists("Salary Slip", {"employee": employee_details.employee, "start_date": self.start_date, "end_date": self.end_date, "docstatus": 1}):
+				emp_list.remove(employee_details)
+
+		return emp_list
+
 	def fill_employee_details(self):
 		self.set('employees', [])
 		employees = self.get_emp_list()
@@ -542,7 +561,7 @@
 					title = _("Creating Salary Slips..."))
 		else:
 			salary_slip_name = frappe.db.sql(
-				'''SELECT 
+				'''SELECT
 						name
 					FROM `tabSalary Slip`
 					WHERE company=%s