From 81c82c8d535dc6ec4ca84eaf87872a31cf12ec4a Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 16 Mar 2022 10:31:34 +0530 Subject: [PATCH 01/16] fix(ux): inform the user about salary slip creation/submission happening in the background --- erpnext/payroll/doctype/payroll_entry/payroll_entry.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py index 54d56f9612..5937e81fed 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py @@ -174,9 +174,11 @@ class PayrollEntry(Document): } ) if len(employees) > 30: - frappe.enqueue(create_salary_slips_for_employees, timeout=600, employees=employees, args=args) + frappe.enqueue(create_salary_slips_for_employees, timeout=600, employees=employees, args=args, publish_progress=False) + frappe.msgprint(_("Salary Slip creation has been queued. It may take a few minutes."), + alert=True, indicator="orange") else: - create_salary_slips_for_employees(employees, args, publish_progress=False) + create_salary_slips_for_employees(employees, args, publish_progress=True) # since this method is called via frm.call this doc needs to be updated manually self.reload() @@ -209,6 +211,8 @@ class PayrollEntry(Document): frappe.enqueue( submit_salary_slips_for_employees, timeout=600, payroll_entry=self, salary_slips=ss_list ) + frappe.msgprint(_("Salary Slip submission has been queued. It may take a few minutes."), + alert=True, indicator="orange") else: submit_salary_slips_for_employees(self, ss_list, publish_progress=False) From ef8164f188005942409123ca6bc136ad29b3c503 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 19 May 2022 20:33:55 +0530 Subject: [PATCH 02/16] refactor: UX for Salary Slip creation and submission via Payroll Entry - Add status for Queued/Failed - log errors and show corrective actions in payroll entry --- .../doctype/payroll_entry/payroll_entry.js | 31 ++- .../doctype/payroll_entry/payroll_entry.json | 50 ++++- .../doctype/payroll_entry/payroll_entry.py | 208 ++++++++++++------ .../payroll_entry/payroll_entry_list.js | 18 ++ 4 files changed, 230 insertions(+), 77 deletions(-) create mode 100644 erpnext/payroll/doctype/payroll_entry/payroll_entry_list.js diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js index 62e183e59c..a33f7665bd 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js @@ -64,6 +64,32 @@ frappe.ui.form.on('Payroll Entry', { if (frm.custom_buttons) frm.clear_custom_buttons(); frm.events.add_context_buttons(frm); } + + if (frm.doc.status == "Failed" && frm.doc.error_message) { + const issue = `issue`; + let process = (cint(frm.doc.salary_slips_created)) ? "submission" : "creation"; + + frm.dashboard.set_headline( + __("Salary Slip {0} failed. You can resolve the {1} and retry {0}.", [process, issue]) + ); + + $("#jump_to_error").on("click", (e) => { + e.preventDefault(); + frappe.utils.scroll_to( + frm.get_field("error_message").$wrapper, + true, + 30 + ); + }); + } + + frappe.realtime.on("completed_salary_slip_creation", function() { + frm.reload_doc(); + }); + + frappe.realtime.on("completed_salary_slip_submission", function() { + frm.reload_doc(); + }); }, get_employee_details: function (frm) { @@ -88,7 +114,7 @@ frappe.ui.form.on('Payroll Entry', { doc: frm.doc, method: "create_salary_slips", callback: function () { - frm.refresh(); + frm.reload_doc(); frm.toolbar.refresh(); } }); @@ -97,7 +123,7 @@ frappe.ui.form.on('Payroll Entry', { 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) { + } else if (frm.doc.salary_slips_created && frm.doc.status != 'Queued') { frm.add_custom_button(__("Submit Salary Slip"), function () { submit_salary_slip(frm); }).addClass("btn-primary"); @@ -331,6 +357,7 @@ const submit_salary_slip = function (frm) { method: 'submit_salary_slips', args: {}, callback: function () { + frm.reload_doc(); frm.events.refresh(frm); }, doc: frm.doc, diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.json b/erpnext/payroll/doctype/payroll_entry/payroll_entry.json index 0444134aa4..17882eb5d9 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.json +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.json @@ -8,11 +8,11 @@ "engine": "InnoDB", "field_order": [ "section_break0", - "column_break0", "posting_date", "payroll_frequency", "company", "column_break1", + "status", "currency", "exchange_rate", "payroll_payable_account", @@ -41,11 +41,14 @@ "cost_center", "account", "payment_account", - "amended_from", "column_break_33", "bank_account", "salary_slips_created", - "salary_slips_submitted" + "salary_slips_submitted", + "failure_details_section", + "error_message", + "section_break_41", + "amended_from" ], "fields": [ { @@ -53,11 +56,6 @@ "fieldtype": "Section Break", "label": "Select Employees" }, - { - "fieldname": "column_break0", - "fieldtype": "Column Break", - "width": "50%" - }, { "default": "Today", "fieldname": "posting_date", @@ -231,6 +229,7 @@ "fieldtype": "Check", "hidden": 1, "label": "Salary Slips Created", + "no_copy": 1, "read_only": 1 }, { @@ -239,6 +238,7 @@ "fieldtype": "Check", "hidden": 1, "label": "Salary Slips Submitted", + "no_copy": 1, "read_only": 1 }, { @@ -284,15 +284,44 @@ "label": "Payroll Payable Account", "options": "Account", "reqd": 1 + }, + { + "collapsible": 1, + "collapsible_depends_on": "error_message", + "depends_on": "eval:doc.status=='Failed';", + "fieldname": "failure_details_section", + "fieldtype": "Section Break", + "label": "Failure Details" + }, + { + "depends_on": "eval:doc.status=='Failed';", + "fieldname": "error_message", + "fieldtype": "Small Text", + "label": "Error Message", + "no_copy": 1, + "read_only": 1 + }, + { + "fieldname": "section_break_41", + "fieldtype": "Section Break" + }, + { + "fieldname": "status", + "fieldtype": "Select", + "label": "Status", + "options": "Draft\nSubmitted\nCancelled\nQueued\nFailed", + "print_hide": 1, + "read_only": 1 } ], "icon": "fa fa-cog", "is_submittable": 1, "links": [], - "modified": "2020-12-17 15:13:17.766210", + "modified": "2022-03-16 12:45:21.662765", "modified_by": "Administrator", "module": "Payroll", "name": "Payroll Entry", + "naming_rule": "Expression (old style)", "owner": "Administrator", "permissions": [ { @@ -308,5 +337,6 @@ } ], "sort_field": "modified", - "sort_order": "DESC" + "sort_order": "DESC", + "states": [] } \ No newline at end of file diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py index 5937e81fed..86be813b91 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py @@ -1,6 +1,7 @@ # Copyright (c) 2017, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt +import json import frappe from dateutil.relativedelta import relativedelta @@ -16,6 +17,7 @@ from frappe.utils import ( comma_and, date_diff, flt, + get_link_to_form, getdate, ) @@ -39,8 +41,10 @@ class PayrollEntry(Document): def validate(self): self.number_of_employees = len(self.employees) + self.set_status() def on_submit(self): + self.set_status(update=True) self.create_salary_slips() def before_submit(self): @@ -49,6 +53,15 @@ class PayrollEntry(Document): if self.validate_employee_attendance(): frappe.throw(_("Cannot Submit, Employees left to mark attendance")) + def set_status(self, status=None, update=True): + if not status: + status = {0: "Draft", 1: "Submitted", 2: "Cancelled"}[self.docstatus or 0] + + if update: + self.db_set("status", status) + else: + self.status = status + def validate_employee_details(self): emp_with_sal_slip = [] for employee_details in self.employees: @@ -77,6 +90,7 @@ class PayrollEntry(Document): ) self.db_set("salary_slips_created", 0) self.db_set("salary_slips_submitted", 0) + self.set_status(update=True) def get_emp_list(self): """ @@ -174,11 +188,21 @@ class PayrollEntry(Document): } ) if len(employees) > 30: - frappe.enqueue(create_salary_slips_for_employees, timeout=600, employees=employees, args=args, publish_progress=False) - frappe.msgprint(_("Salary Slip creation has been queued. It may take a few minutes."), - alert=True, indicator="orange") + self.db_set("status", "Queued") + frappe.enqueue( + create_salary_slips_for_employees, + timeout=600, + employees=employees, + args=args, + publish_progress=False, + ) + frappe.msgprint( + _("Salary Slip creation is queued. It may take a few minutes"), + alert=True, + indicator="blue", + ) else: - create_salary_slips_for_employees(employees, args, publish_progress=True) + create_salary_slips_for_employees(employees, args, publish_progress=False) # since this method is called via frm.call this doc needs to be updated manually self.reload() @@ -208,11 +232,19 @@ class PayrollEntry(Document): self.check_permission("write") ss_list = self.get_sal_slip_list(ss_status=0) if len(ss_list) > 30: + self.db_set("status", "Queued") frappe.enqueue( - submit_salary_slips_for_employees, timeout=600, payroll_entry=self, salary_slips=ss_list + submit_salary_slips_for_employees, + timeout=600, + payroll_entry=self, + salary_slips=ss_list, + publish_progress=False, + ) + frappe.msgprint( + _("Salary Slip submission is queued. It may take a few minutes"), + alert=True, + indicator="blue", ) - frappe.msgprint(_("Salary Slip submission has been queued. It may take a few minutes."), - alert=True, indicator="orange") else: submit_salary_slips_for_employees(self, ss_list, publish_progress=False) @@ -227,7 +259,11 @@ class PayrollEntry(Document): ) if not account: - frappe.throw(_("Please set account in Salary Component {0}").format(salary_component)) + frappe.throw( + _("Please set account in Salary Component {0}").format( + get_link_to_form("Salary Component", salary_component) + ) + ) return account @@ -784,37 +820,81 @@ def payroll_entry_has_bank_entries(name): return response +def log_payroll_failure(process, payroll_entry, error): + error_log = frappe.log_error( + title=_("Salary Slip {0} failed for Payroll Entry {1}").format(process, payroll_entry.name) + ) + message_log = frappe.message_log.pop() if frappe.message_log else str(error) + + try: + error_message = json.loads(message_log).get("message") + except Exception: + error_message = message_log + + error_message += "\n" + _("Check Error Log {0} for more details.").format( + get_link_to_form("Error Log", error_log.name) + ) + + payroll_entry.db_set({"error_message": error_message, "status": "Failed"}) + + def create_salary_slips_for_employees(employees, args, publish_progress=True): - salary_slips_exists_for = get_existing_salary_slips(employees, args) - count = 0 - salary_slips_not_created = [] - for emp in employees: - if emp not in salary_slips_exists_for: - args.update({"doctype": "Salary Slip", "employee": emp}) - ss = frappe.get_doc(args) - ss.insert() - count += 1 - if publish_progress: - frappe.publish_progress( - count * 100 / len(set(employees) - set(salary_slips_exists_for)), - title=_("Creating Salary Slips..."), - ) + try: + frappe.db.savepoint("salary_slip_creation") + payroll_entry = frappe.get_doc("Payroll Entry", args.payroll_entry) + salary_slips_exist_for = get_existing_salary_slips(employees, args) + count = 0 - else: - salary_slips_not_created.append(emp) + for emp in employees: + if emp not in salary_slips_exist_for: + args.update({"doctype": "Salary Slip", "employee": emp}) + frappe.get_doc(args).insert() - payroll_entry = frappe.get_doc("Payroll Entry", args.payroll_entry) - payroll_entry.db_set("salary_slips_created", 1) - payroll_entry.notify_update() + count += 1 + if publish_progress: + frappe.publish_progress( + count * 100 / len(set(employees) - set(salary_slips_exist_for)), + title=_("Creating Salary Slips..."), + ) - if salary_slips_not_created: + payroll_entry.db_set({"status": "Submitted", "salary_slips_created": 1}) + + if salary_slips_exist_for: + frappe.msgprint( + _( + "Salary Slips already exist for employees {}, and will not be processed by this payroll." + ).format(frappe.bold(", ".join(emp for emp in salary_slips_exist_for))), + title=_("Message"), + indicator="orange", + ) + + except Exception as e: + frappe.db.rollback(save_point="salary_slip_creation") + log_payroll_failure("creation", payroll_entry, e) + + finally: + frappe.db.commit() + frappe.publish_realtime("completed_salary_slip_creation") + + +def show_payroll_submission_status(submitted, not_submitted, salary_slip): + if not submitted and not not_submitted: frappe.msgprint( _( - "Salary Slips already exists for employees {}, and will not be processed by this payroll." - ).format(frappe.bold(", ".join([emp for emp in salary_slips_not_created]))), - title=_("Message"), - indicator="orange", + "No salary slip found to submit for the above selected criteria OR salary slip already submitted" + ) ) + return + + if submitted: + frappe.msgprint( + _("Salary Slip submitted for period from {0} to {1}").format( + salary_slip.start_date, salary_slip.end_date + ) + ) + + if not_submitted: + frappe.msgprint(_("Could not submit some Salary Slips")) def get_existing_salary_slips(employees, args): @@ -831,45 +911,43 @@ def get_existing_salary_slips(employees, args): def submit_salary_slips_for_employees(payroll_entry, salary_slips, publish_progress=True): - submitted_ss = [] - not_submitted_ss = [] - frappe.flags.via_payroll_entry = True + try: + frappe.db.savepoint("salary_slip_submission") - count = 0 - for ss in salary_slips: - ss_obj = frappe.get_doc("Salary Slip", ss[0]) - if ss_obj.net_pay < 0: - not_submitted_ss.append(ss[0]) - else: - try: - ss_obj.submit() - submitted_ss.append(ss_obj) - except frappe.ValidationError: - not_submitted_ss.append(ss[0]) + submitted = [] + not_submitted = [] + frappe.flags.via_payroll_entry = True + count = 0 - count += 1 - if publish_progress: - frappe.publish_progress(count * 100 / len(salary_slips), title=_("Submitting Salary Slips...")) - if submitted_ss: - payroll_entry.make_accrual_jv_entry() - frappe.msgprint( - _("Salary Slip submitted for period from {0} to {1}").format(ss_obj.start_date, ss_obj.end_date) - ) + for entry in salary_slips: + salary_slip = frappe.get_doc("Salary Slip", entry[0]) + if salary_slip.net_pay < 0: + not_submitted.append(entry[0]) + else: + try: + salary_slip.submit() + submitted.append(salary_slip) + except frappe.ValidationError: + not_submitted.append(entry[0]) - payroll_entry.email_salary_slip(submitted_ss) + count += 1 + if publish_progress: + frappe.publish_progress(count * 100 / len(salary_slips), title=_("Submitting Salary Slips...")) - payroll_entry.db_set("salary_slips_submitted", 1) - payroll_entry.notify_update() + if submitted: + payroll_entry.make_accrual_jv_entry() + payroll_entry.email_salary_slip(submitted) + payroll_entry.db_set({"salary_slips_submitted": 1, "status": "Submitted"}) - if not submitted_ss and not not_submitted_ss: - frappe.msgprint( - _( - "No salary slip found to submit for the above selected criteria OR salary slip already submitted" - ) - ) + show_payroll_submission_status(submitted, not_submitted, salary_slip) - if not_submitted_ss: - frappe.msgprint(_("Could not submit some Salary Slips")) + except Exception as e: + frappe.db.rollback(save_point="salary_slip_submission") + log_payroll_failure("submission", payroll_entry, e) + + finally: + frappe.db.commit() + frappe.publish_realtime("completed_salary_slip_submission") frappe.flags.via_payroll_entry = False diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry_list.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry_list.js new file mode 100644 index 0000000000..56390b79d8 --- /dev/null +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry_list.js @@ -0,0 +1,18 @@ +// Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +// License: GNU General Public License v3. See license.txt + +// render +frappe.listview_settings['Payroll Entry'] = { + has_indicator_for_draft: 1, + get_indicator: function(doc) { + var status_color = { + 'Draft': 'red', + 'Submitted': 'blue', + 'Queued': 'orange', + 'Failed': 'red', + 'Cancelled': 'red' + + }; + return [__(doc.status), status_color[doc.status], 'status,=,'+doc.status]; + } +}; From 7d4872aedd254efe85f61d88d98a3285859da11d Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 19 May 2022 20:35:56 +0530 Subject: [PATCH 03/16] patch: set payroll entry status --- erpnext/patches.txt | 1 + .../patches/v13_0/set_payroll_entry_status.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 erpnext/patches/v13_0/set_payroll_entry_status.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 4d9a7e06bf..e710aa3389 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -372,3 +372,4 @@ erpnext.patches.v14_0.discount_accounting_separation erpnext.patches.v14_0.delete_employee_transfer_property_doctype erpnext.patches.v13_0.create_accounting_dimensions_in_orders erpnext.patches.v13_0.set_per_billed_in_return_delivery_note +erpnext.patches.v13_0.set_payroll_entry_status diff --git a/erpnext/patches/v13_0/set_payroll_entry_status.py b/erpnext/patches/v13_0/set_payroll_entry_status.py new file mode 100644 index 0000000000..97adff9295 --- /dev/null +++ b/erpnext/patches/v13_0/set_payroll_entry_status.py @@ -0,0 +1,16 @@ +import frappe +from frappe.query_builder import Case + + +def execute(): + PayrollEntry = frappe.qb.DocType("Payroll Entry") + + ( + frappe.qb.update(PayrollEntry).set( + "status", + Case() + .when(PayrollEntry.docstatus == 0, "Draft") + .when(PayrollEntry.docstatus == 1, "Submitted") + .else_("Cancelled"), + ) + ).run() From de5515799732cbfb06a9e05ee3951f180a34e841 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 30 May 2022 12:47:26 +0530 Subject: [PATCH 04/16] chore: rename method `get_salary_component_account` method to `set` - since it doesn't return any value --- .../payroll/doctype/payroll_entry/test_payroll_entry.py | 8 ++++---- erpnext/payroll/doctype/salary_slip/test_salary_slip.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py index fda0fcf8be..3fffeb8966 100644 --- a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py @@ -22,10 +22,10 @@ from erpnext.loan_management.doctype.process_loan_interest_accrual.process_loan_ from erpnext.payroll.doctype.payroll_entry.payroll_entry import get_end_date, get_start_end_dates from erpnext.payroll.doctype.salary_slip.test_salary_slip import ( create_account, - get_salary_component_account, make_deduction_salary_component, make_earning_salary_component, make_employee_salary_slip, + set_salary_component_account, ) from erpnext.payroll.doctype.salary_structure.test_salary_structure import ( create_salary_structure_assignment, @@ -66,7 +66,7 @@ class TestPayrollEntry(unittest.TestCase): if not frappe.db.get_value( "Salary Component Account", {"parent": data.name, "company": company}, "name" ): - get_salary_component_account(data.name) + set_salary_component_account(data.name) employee = frappe.db.get_value("Employee", {"company": company}) company_doc = frappe.get_doc("Company", company) @@ -95,7 +95,7 @@ class TestPayrollEntry(unittest.TestCase): if not frappe.db.get_value( "Salary Component Account", {"parent": data.name, "company": company}, "name" ): - get_salary_component_account(data.name) + set_salary_component_account(data.name) company_doc = frappe.get_doc("Company", company) salary_structure = make_salary_structure( @@ -148,7 +148,7 @@ class TestPayrollEntry(unittest.TestCase): if not frappe.db.get_value( "Salary Component Account", {"parent": data.name, "company": "_Test Company"}, "name" ): - get_salary_component_account(data.name) + set_salary_component_account(data.name) if not frappe.db.exists("Department", "cc - _TC"): frappe.get_doc( diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 1bc3741922..91d335274e 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1046,10 +1046,10 @@ def make_salary_component(salary_components, test_tax, company_list=None): doc.update(salary_component) doc.insert() - get_salary_component_account(doc, company_list) + set_salary_component_account(doc, company_list) -def get_salary_component_account(sal_comp, company_list=None): +def set_salary_component_account(sal_comp, company_list=None): company = erpnext.get_default_company() if company_list and company not in company_list: From 42f6bca9354d07855573235ab8508c49e5ba9cac Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 30 May 2022 16:04:07 +0530 Subject: [PATCH 05/16] fix: reset Error Message on successful operation and fix status update on submit/cancel --- .../doctype/payroll_entry/payroll_entry.py | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py index 86be813b91..266621d4d1 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py @@ -44,7 +44,7 @@ class PayrollEntry(Document): self.set_status() def on_submit(self): - self.set_status(update=True) + self.set_status(update=True, status="Submitted") self.create_salary_slips() def before_submit(self): @@ -90,7 +90,8 @@ class PayrollEntry(Document): ) self.db_set("salary_slips_created", 0) self.db_set("salary_slips_submitted", 0) - self.set_status(update=True) + self.set_status(update=True, status="Cancelled") + self.db_set("error_message", "") def get_emp_list(self): """ @@ -187,7 +188,7 @@ class PayrollEntry(Document): "currency": self.currency, } ) - if len(employees) > 30: + if len(employees) > 30 or frappe.flags.enqueue_payroll_entry: self.db_set("status", "Queued") frappe.enqueue( create_salary_slips_for_employees, @@ -230,14 +231,14 @@ class PayrollEntry(Document): @frappe.whitelist() def submit_salary_slips(self): self.check_permission("write") - ss_list = self.get_sal_slip_list(ss_status=0) - if len(ss_list) > 30: + salary_slips = self.get_sal_slip_list(ss_status=0) + if len(salary_slips) > 30 or frappe.flags.enqueue_payroll_entry: self.db_set("status", "Queued") frappe.enqueue( submit_salary_slips_for_employees, timeout=600, payroll_entry=self, - salary_slips=ss_list, + salary_slips=salary_slips, publish_progress=False, ) frappe.msgprint( @@ -246,7 +247,7 @@ class PayrollEntry(Document): indicator="blue", ) else: - submit_salary_slips_for_employees(self, ss_list, publish_progress=False) + submit_salary_slips_for_employees(self, salary_slips, publish_progress=False) def email_salary_slip(self, submitted_ss): if frappe.db.get_single_value("Payroll Settings", "email_salary_slip_to_employee"): @@ -857,7 +858,7 @@ def create_salary_slips_for_employees(employees, args, publish_progress=True): title=_("Creating Salary Slips..."), ) - payroll_entry.db_set({"status": "Submitted", "salary_slips_created": 1}) + payroll_entry.db_set({"status": "Submitted", "salary_slips_created": 1, "error_message": ""}) if salary_slips_exist_for: frappe.msgprint( @@ -873,7 +874,7 @@ def create_salary_slips_for_employees(employees, args, publish_progress=True): log_payroll_failure("creation", payroll_entry, e) finally: - frappe.db.commit() + frappe.db.commit() # nosemgrep frappe.publish_realtime("completed_salary_slip_creation") @@ -937,7 +938,7 @@ def submit_salary_slips_for_employees(payroll_entry, salary_slips, publish_progr if submitted: payroll_entry.make_accrual_jv_entry() payroll_entry.email_salary_slip(submitted) - payroll_entry.db_set({"salary_slips_submitted": 1, "status": "Submitted"}) + payroll_entry.db_set({"salary_slips_submitted": 1, "status": "Submitted", "error_message": ""}) show_payroll_submission_status(submitted, not_submitted, salary_slip) @@ -946,7 +947,7 @@ def submit_salary_slips_for_employees(payroll_entry, salary_slips, publish_progr log_payroll_failure("submission", payroll_entry, e) finally: - frappe.db.commit() + frappe.db.commit() # nosemgrep frappe.publish_realtime("completed_salary_slip_submission") frappe.flags.via_payroll_entry = False From 78c39e947bf747032c935dab6d76092836fa0e61 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 30 May 2022 16:07:19 +0530 Subject: [PATCH 06/16] test: Salary Slip operations queuing, failure, and payroll entry status - fix multicurrency test, remove redundant doc creation --- .../payroll_entry/test_payroll_entry.py | 172 ++++++++++++++---- .../salary_structure/test_salary_structure.py | 3 - 2 files changed, 137 insertions(+), 38 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py index 3fffeb8966..5c68bd35ef 100644 --- a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py @@ -5,6 +5,7 @@ import unittest import frappe from dateutil.relativedelta import relativedelta +from frappe.tests.utils import FrappeTestCase from frappe.utils import add_months import erpnext @@ -35,14 +36,12 @@ from erpnext.payroll.doctype.salary_structure.test_salary_structure import ( test_dependencies = ["Holiday List"] -class TestPayrollEntry(unittest.TestCase): - @classmethod - def setUpClass(cls): +class TestPayrollEntry(FrappeTestCase): + def setUp(self): frappe.db.set_value( "Company", erpnext.get_default_company(), "default_holiday_list", "_Test Holiday List" ) - def setUp(self): for dt in [ "Salary Slip", "Salary Component", @@ -88,40 +87,40 @@ class TestPayrollEntry(unittest.TestCase): currency=company_doc.default_currency, ) - def test_multi_currency_payroll_entry(self): # pylint: disable=no-self-use - company = erpnext.get_default_company() - employee = make_employee("test_muti_currency_employee@payroll.com", company=company) + def test_multi_currency_payroll_entry(self): + company = frappe.get_doc("Company", "_Test Company") + employee = make_employee( + "test_muti_currency_employee@payroll.com", company=company.name, department="Accounts - _TC" + ) + for data in frappe.get_all("Salary Component", fields=["name"]): if not frappe.db.get_value( - "Salary Component Account", {"parent": data.name, "company": company}, "name" + "Salary Component Account", {"parent": data.name, "company": company.name}, "name" ): set_salary_component_account(data.name) - company_doc = frappe.get_doc("Company", company) - salary_structure = make_salary_structure( - "_Test Multi Currency Salary Structure", "Monthly", company=company, currency="USD" - ) - create_salary_structure_assignment( - employee, salary_structure.name, company=company, currency="USD" - ) - frappe.db.sql( - """delete from `tabSalary Slip` where employee=%s""", - (frappe.db.get_value("Employee", {"user_id": "test_muti_currency_employee@payroll.com"})), - ) - salary_slip = get_salary_slip( - "test_muti_currency_employee@payroll.com", "Monthly", "_Test Multi Currency Salary Structure" + salary_struct = make_salary_structure( + "_Test Multi Currency Salary Structure", + "Monthly", + employee, + currency="USD", + company=company.name, ) + + frappe.db.delete("Salary Slip", {"employee": employee}) dates = get_start_end_dates("Monthly", nowdate()) payroll_entry = make_payroll_entry( start_date=dates.start_date, end_date=dates.end_date, - payable_account=company_doc.default_payroll_payable_account, + payable_account=company.default_payroll_payable_account, currency="USD", exchange_rate=70, + company=company.name, ) payroll_entry.make_payment_entry() - salary_slip.load_from_db() + salary_slip = frappe.db.get_value("Salary Slip", {"payroll_entry": payroll_entry.name}) + salary_slip = frappe.get_doc("Salary Slip", salary_slip) payroll_je = salary_slip.journal_entry if payroll_je: @@ -143,7 +142,7 @@ class TestPayrollEntry(unittest.TestCase): self.assertEqual(salary_slip.base_net_pay, payment_entry[0].total_debit) self.assertEqual(salary_slip.base_net_pay, payment_entry[0].total_credit) - def test_payroll_entry_with_employee_cost_center(self): # pylint: disable=no-self-use + def test_payroll_entry_with_employee_cost_center(self): for data in frappe.get_all("Salary Component", fields=["name"]): if not frappe.db.get_value( "Salary Component Account", {"parent": data.name, "company": "_Test Company"}, "name" @@ -356,8 +355,114 @@ class TestPayrollEntry(unittest.TestCase): if salary_slip.docstatus == 0: frappe.delete_doc("Salary Slip", name) + def test_salary_slip_operation_queueing(self): + # setup + company = erpnext.get_default_company() + company_doc = frappe.get_doc("Company", company) + employee = frappe.db.get_value("Employee", {"company": company}) + make_salary_structure( + "_Test Salary Structure", + "Monthly", + employee, + company=company, + currency=company_doc.default_currency, + ) -def make_payroll_entry(**args): + # enqueue salary slip creation via payroll entry + # Payroll Entry status should change to Queued + dates = get_start_end_dates("Monthly", nowdate()) + payroll_entry = get_payroll_entry_data( + start_date=dates.start_date, + end_date=dates.end_date, + payable_account=company_doc.default_payroll_payable_account, + currency=company_doc.default_currency, + ) + frappe.flags.enqueue_payroll_entry = True + payroll_entry.create_salary_slips() + payroll_entry.reload() + + self.assertEqual(payroll_entry.status, "Queued") + frappe.flags.enqueue_payroll_entry = False + + def test_salary_slip_operation_failure(self): + # setup + company = erpnext.get_default_company() + company_doc = frappe.get_doc("Company", company) + employee = frappe.db.get_value("Employee", {"company": company}) + salary_structure = make_salary_structure( + "_Test Salary Structure", + "Monthly", + employee, + company=company, + currency=company_doc.default_currency, + ) + + # reset account in component to test submission failure + component = frappe.get_doc("Salary Component", salary_structure.earnings[0].salary_component) + component.accounts = [] + component.save() + + # salary slip submission via payroll entry + # Payroll Entry status should change to Failed because of the missing account setup + dates = get_start_end_dates("Monthly", nowdate()) + payroll_entry = get_payroll_entry_data( + start_date=dates.start_date, + end_date=dates.end_date, + payable_account=company_doc.default_payroll_payable_account, + currency=company_doc.default_currency, + ) + payroll_entry.create_salary_slips() + payroll_entry.submit_salary_slips() + + payroll_entry.reload() + self.assertEqual(payroll_entry.status, "Failed") + self.assertIsNotNone(payroll_entry.error_message) + + # set accounts + for data in frappe.get_all("Salary Component", fields=["name"]): + if not frappe.db.get_value( + "Salary Component Account", {"parent": data.name, "company": company}, "name" + ): + set_salary_component_account(data.name, company_list=[company]) + + # Payroll Entry successful, status should change to Submitted + payroll_entry.submit_salary_slips() + payroll_entry.reload() + self.assertEqual(payroll_entry.status, "Submitted") + self.assertEqual(payroll_entry.error_message, "") + + def test_payroll_entry_status(self): + company = erpnext.get_default_company() + for data in frappe.get_all("Salary Component", fields=["name"]): + if not frappe.db.get_value( + "Salary Component Account", {"parent": data.name, "company": company}, "name" + ): + set_salary_component_account(data.name) + + employee = frappe.db.get_value("Employee", {"company": company}) + company_doc = frappe.get_doc("Company", company) + make_salary_structure( + "_Test Salary Structure", + "Monthly", + employee, + company=company, + currency=company_doc.default_currency, + ) + dates = get_start_end_dates("Monthly", nowdate()) + payroll_entry = get_payroll_entry_data( + start_date=dates.start_date, + end_date=dates.end_date, + payable_account=company_doc.default_payroll_payable_account, + currency=company_doc.default_currency, + ) + payroll_entry.submit() + self.assertEqual(payroll_entry.status, "Submitted") + + payroll_entry.cancel() + self.assertEqual(payroll_entry.status, "Cancelled") + + +def get_payroll_entry_data(**args): args = frappe._dict(args) payroll_entry = frappe.new_doc("Payroll Entry") @@ -381,7 +486,13 @@ def make_payroll_entry(**args): payroll_entry.fill_employee_details() payroll_entry.save() - payroll_entry.create_salary_slips() + + return payroll_entry + + +def make_payroll_entry(**args): + payroll_entry = get_payroll_entry_data(**args) + payroll_entry.submit() payroll_entry.submit_salary_slips() if payroll_entry.get_sal_slip_list(ss_status=1): payroll_entry.make_payment_entry() @@ -421,12 +532,3 @@ def make_holiday(holiday_list_name): ).insert() return holiday_list_name - - -def get_salary_slip(user, period, salary_structure): - salary_slip = make_employee_salary_slip(user, period, salary_structure) - salary_slip.exchange_rate = 70 - salary_slip.calculate_net_pay() - salary_slip.db_update() - - return salary_slip diff --git a/erpnext/payroll/doctype/salary_structure/test_salary_structure.py b/erpnext/payroll/doctype/salary_structure/test_salary_structure.py index e9b5ed2261..9a0e8188f8 100644 --- a/erpnext/payroll/doctype/salary_structure/test_salary_structure.py +++ b/erpnext/payroll/doctype/salary_structure/test_salary_structure.py @@ -169,9 +169,6 @@ def make_salary_structure( payroll_period=None, include_flexi_benefits=False, ): - if test_tax: - frappe.db.sql("""delete from `tabSalary Structure` where name=%s""", (salary_structure)) - if frappe.db.exists("Salary Structure", salary_structure): frappe.db.delete("Salary Structure", salary_structure) From 653d6341d46fcc6e2c49167a0ecfc7ccb1003d35 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 1 Jun 2022 12:14:42 +0530 Subject: [PATCH 07/16] refactor: clean-up and commonify payroll entry test setups --- .../payroll_entry/test_payroll_entry.py | 295 ++++++++---------- 1 file changed, 126 insertions(+), 169 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py index 5c68bd35ef..47b9962912 100644 --- a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py @@ -25,7 +25,6 @@ from erpnext.payroll.doctype.salary_slip.test_salary_slip import ( create_account, make_deduction_salary_component, make_earning_salary_component, - make_employee_salary_slip, set_salary_component_account, ) from erpnext.payroll.doctype.salary_structure.test_salary_structure import ( @@ -38,10 +37,6 @@ test_dependencies = ["Holiday List"] class TestPayrollEntry(FrappeTestCase): def setUp(self): - frappe.db.set_value( - "Company", erpnext.get_default_company(), "default_holiday_list", "_Test Holiday List" - ) - for dt in [ "Salary Slip", "Salary Component", @@ -51,76 +46,79 @@ class TestPayrollEntry(FrappeTestCase): "Salary Structure Assignment", "Payroll Employee Detail", "Additional Salary", + "Loan", ]: - frappe.db.sql("delete from `tab%s`" % dt) + frappe.db.delete(dt) make_earning_salary_component(setup=True, company_list=["_Test Company"]) make_deduction_salary_component(setup=True, test_tax=False, company_list=["_Test Company"]) + frappe.db.set_value("Company", "_Test Company", "default_holiday_list", "_Test Holiday List") frappe.db.set_value("Payroll Settings", None, "email_salary_slip_to_employee", 0) - def test_payroll_entry(self): # pylint: disable=no-self-use + # set default payable account + default_account = frappe.db.get_value( + "Company", "_Test Company", "default_payroll_payable_account" + ) + if not default_account or default_account != "_Test Payroll Payable - _TC": + create_account( + account_name="_Test Payroll Payable", + company="_Test Company", + parent_account="Current Liabilities - _TC", + account_type="Payable", + ) + frappe.db.set_value( + "Company", "_Test Company", "default_payroll_payable_account", "_Test Payroll Payable - _TC" + ) + + def test_payroll_entry(self): + company = frappe.get_doc("Company", "_Test Company") + employee = frappe.db.get_value("Employee", {"company": "_Test Company"}) + setup_salary_structure(employee, company) + + dates = get_start_end_dates("Monthly", nowdate()) + make_payroll_entry( + start_date=dates.start_date, + end_date=dates.end_date, + payable_account=company.default_payroll_payable_account, + currency=company.default_currency, + company=company.name, + ) + + def test_multi_currency_payroll_entry(self): company = erpnext.get_default_company() + employee = make_employee("test_muti_currency_employee@payroll.com", company=company) for data in frappe.get_all("Salary Component", fields=["name"]): if not frappe.db.get_value( "Salary Component Account", {"parent": data.name, "company": company}, "name" ): - set_salary_component_account(data.name) + get_salary_component_account(data.name) - employee = frappe.db.get_value("Employee", {"company": company}) company_doc = frappe.get_doc("Company", company) - make_salary_structure( - "_Test Salary Structure", - "Monthly", - employee, - company=company, - currency=company_doc.default_currency, + salary_structure = make_salary_structure( + "_Test Multi Currency Salary Structure", "Monthly", company=company, currency="USD" ) - dates = get_start_end_dates("Monthly", nowdate()) - if not frappe.db.get_value( - "Salary Slip", {"start_date": dates.start_date, "end_date": dates.end_date} - ): - make_payroll_entry( - start_date=dates.start_date, - end_date=dates.end_date, - payable_account=company_doc.default_payroll_payable_account, - currency=company_doc.default_currency, - ) - - def test_multi_currency_payroll_entry(self): - company = frappe.get_doc("Company", "_Test Company") - employee = make_employee( - "test_muti_currency_employee@payroll.com", company=company.name, department="Accounts - _TC" + create_salary_structure_assignment( + employee, salary_structure.name, company=company, currency="USD" ) - - for data in frappe.get_all("Salary Component", fields=["name"]): - if not frappe.db.get_value( - "Salary Component Account", {"parent": data.name, "company": company.name}, "name" - ): - set_salary_component_account(data.name) - - salary_struct = make_salary_structure( - "_Test Multi Currency Salary Structure", - "Monthly", - employee, - currency="USD", - company=company.name, + frappe.db.sql( + """delete from `tabSalary Slip` where employee=%s""", + (frappe.db.get_value("Employee", {"user_id": "test_muti_currency_employee@payroll.com"})), + ) + salary_slip = get_salary_slip( + "test_muti_currency_employee@payroll.com", "Monthly", "_Test Multi Currency Salary Structure" ) - - frappe.db.delete("Salary Slip", {"employee": employee}) dates = get_start_end_dates("Monthly", nowdate()) payroll_entry = make_payroll_entry( start_date=dates.start_date, end_date=dates.end_date, - payable_account=company.default_payroll_payable_account, + payable_account=company_doc.default_payroll_payable_account, currency="USD", exchange_rate=70, - company=company.name, ) payroll_entry.make_payment_entry() - salary_slip = frappe.db.get_value("Salary Slip", {"payroll_entry": payroll_entry.name}) - salary_slip = frappe.get_doc("Salary Slip", salary_slip) + salary_slip.load_from_db() payroll_je = salary_slip.journal_entry if payroll_je: @@ -143,22 +141,11 @@ class TestPayrollEntry(FrappeTestCase): self.assertEqual(salary_slip.base_net_pay, payment_entry[0].total_credit) def test_payroll_entry_with_employee_cost_center(self): - for data in frappe.get_all("Salary Component", fields=["name"]): - if not frappe.db.get_value( - "Salary Component Account", {"parent": data.name, "company": "_Test Company"}, "name" - ): - set_salary_component_account(data.name) - if not frappe.db.exists("Department", "cc - _TC"): frappe.get_doc( {"doctype": "Department", "department_name": "cc", "company": "_Test Company"} ).insert() - frappe.db.sql("""delete from `tabEmployee` where employee_name='test_employee1@example.com' """) - frappe.db.sql("""delete from `tabEmployee` where employee_name='test_employee2@example.com' """) - frappe.db.sql("""delete from `tabSalary Structure` where name='_Test Salary Structure 1' """) - frappe.db.sql("""delete from `tabSalary Structure` where name='_Test Salary Structure 2' """) - employee1 = make_employee( "test_employee1@example.com", payroll_cost_center="_Test Cost Center - _TC", @@ -169,38 +156,15 @@ class TestPayrollEntry(FrappeTestCase): "test_employee2@example.com", department="cc - _TC", company="_Test Company" ) - if not frappe.db.exists("Account", "_Test Payroll Payable - _TC"): - create_account( - account_name="_Test Payroll Payable", - company="_Test Company", - parent_account="Current Liabilities - _TC", - account_type="Payable", - ) + company = frappe.get_doc("Company", "_Test Company") + setup_salary_structure(employee1, company) - if ( - not frappe.db.get_value("Company", "_Test Company", "default_payroll_payable_account") - or frappe.db.get_value("Company", "_Test Company", "default_payroll_payable_account") - != "_Test Payroll Payable - _TC" - ): - frappe.db.set_value( - "Company", "_Test Company", "default_payroll_payable_account", "_Test Payroll Payable - _TC" - ) - currency = frappe.db.get_value("Company", "_Test Company", "default_currency") - - make_salary_structure( - "_Test Salary Structure 1", - "Monthly", - employee1, - company="_Test Company", - currency=currency, - test_tax=False, - ) ss = make_salary_structure( "_Test Salary Structure 2", "Monthly", employee2, company="_Test Company", - currency=currency, + currency=company.default_currency, test_tax=False, ) @@ -219,42 +183,38 @@ class TestPayrollEntry(FrappeTestCase): ssa_doc.append( "payroll_cost_centers", {"cost_center": "_Test Cost Center 2 - _TC", "percentage": 40} ) - ssa_doc.save() dates = get_start_end_dates("Monthly", nowdate()) - if not frappe.db.get_value( - "Salary Slip", {"start_date": dates.start_date, "end_date": dates.end_date} - ): - pe = make_payroll_entry( - start_date=dates.start_date, - end_date=dates.end_date, - payable_account="_Test Payroll Payable - _TC", - currency=frappe.db.get_value("Company", "_Test Company", "default_currency"), - department="cc - _TC", - company="_Test Company", - payment_account="Cash - _TC", - cost_center="Main - _TC", - ) - je = frappe.db.get_value("Salary Slip", {"payroll_entry": pe.name}, "journal_entry") - je_entries = frappe.db.sql( - """ - select account, cost_center, debit, credit - from `tabJournal Entry Account` - where parent=%s - order by account, cost_center - """, - je, - ) - expected_je = ( - ("_Test Payroll Payable - _TC", "Main - _TC", 0.0, 155600.0), - ("Salary - _TC", "_Test Cost Center - _TC", 124800.0, 0.0), - ("Salary - _TC", "_Test Cost Center 2 - _TC", 31200.0, 0.0), - ("Salary Deductions - _TC", "_Test Cost Center - _TC", 0.0, 320.0), - ("Salary Deductions - _TC", "_Test Cost Center 2 - _TC", 0.0, 80.0), - ) + pe = make_payroll_entry( + start_date=dates.start_date, + end_date=dates.end_date, + payable_account="_Test Payroll Payable - _TC", + currency=frappe.db.get_value("Company", "_Test Company", "default_currency"), + department="cc - _TC", + company="_Test Company", + payment_account="Cash - _TC", + cost_center="Main - _TC", + ) + je = frappe.db.get_value("Salary Slip", {"payroll_entry": pe.name}, "journal_entry") + je_entries = frappe.db.sql( + """ + select account, cost_center, debit, credit + from `tabJournal Entry Account` + where parent=%s + order by account, cost_center + """, + je, + ) + expected_je = ( + ("_Test Payroll Payable - _TC", "Main - _TC", 0.0, 155600.0), + ("Salary - _TC", "_Test Cost Center - _TC", 124800.0, 0.0), + ("Salary - _TC", "_Test Cost Center 2 - _TC", 31200.0, 0.0), + ("Salary Deductions - _TC", "_Test Cost Center - _TC", 0.0, 320.0), + ("Salary Deductions - _TC", "_Test Cost Center 2 - _TC", 0.0, 80.0), + ) - self.assertEqual(je_entries, expected_je) + self.assertEqual(je_entries, expected_je) def test_get_end_date(self): self.assertEqual(get_end_date("2017-01-01", "monthly"), {"end_date": "2017-01-31"}) @@ -267,31 +227,22 @@ class TestPayrollEntry(FrappeTestCase): self.assertEqual(get_end_date("2017-02-15", "daily"), {"end_date": "2017-02-15"}) def test_loan(self): - branch = "Test Employee Branch" - applicant = make_employee("test_employee@loan.com", company="_Test Company") company = "_Test Company" - holiday_list = make_holiday("test holiday for loan") - - company_doc = frappe.get_doc("Company", company) - if not company_doc.default_payroll_payable_account: - company_doc.default_payroll_payable_account = frappe.db.get_value( - "Account", {"company": company, "root_type": "Liability", "account_type": ""}, "name" - ) - company_doc.save() + branch = "Test Employee Branch" if not frappe.db.exists("Branch", branch): frappe.get_doc({"doctype": "Branch", "branch": branch}).insert() + holiday_list = make_holiday("test holiday for loan") - employee_doc = frappe.get_doc("Employee", applicant) - employee_doc.branch = branch - employee_doc.holiday_list = holiday_list - employee_doc.save() + applicant = make_employee( + "test_employee@loan.com", company="_Test Company", branch=branch, holiday_list=holiday_list + ) + company_doc = frappe.get_doc("Company", company) - salary_structure = "Test Salary Structure for Loan" make_salary_structure( - salary_structure, + "Test Salary Structure for Loan", "Monthly", - employee=employee_doc.name, + employee=applicant, company="_Test Company", currency=company_doc.default_currency, ) @@ -352,21 +303,11 @@ class TestPayrollEntry(FrappeTestCase): self.assertEqual(row.principal_amount, principal_amount) self.assertEqual(row.total_payment, interest_amount + principal_amount) - if salary_slip.docstatus == 0: - frappe.delete_doc("Salary Slip", name) - def test_salary_slip_operation_queueing(self): - # setup - company = erpnext.get_default_company() + company = "_Test Company" company_doc = frappe.get_doc("Company", company) employee = frappe.db.get_value("Employee", {"company": company}) - make_salary_structure( - "_Test Salary Structure", - "Monthly", - employee, - company=company, - currency=company_doc.default_currency, - ) + setup_salary_structure(employee, company_doc) # enqueue salary slip creation via payroll entry # Payroll Entry status should change to Queued @@ -376,6 +317,7 @@ class TestPayrollEntry(FrappeTestCase): end_date=dates.end_date, payable_account=company_doc.default_payroll_payable_account, currency=company_doc.default_currency, + company=company_doc.name, ) frappe.flags.enqueue_payroll_entry = True payroll_entry.create_salary_slips() @@ -385,10 +327,10 @@ class TestPayrollEntry(FrappeTestCase): frappe.flags.enqueue_payroll_entry = False def test_salary_slip_operation_failure(self): - # setup - company = erpnext.get_default_company() + company = "_Test Company" company_doc = frappe.get_doc("Company", company) employee = frappe.db.get_value("Employee", {"company": company}) + salary_structure = make_salary_structure( "_Test Salary Structure", "Monthly", @@ -410,6 +352,7 @@ class TestPayrollEntry(FrappeTestCase): end_date=dates.end_date, payable_account=company_doc.default_payroll_payable_account, currency=company_doc.default_currency, + company=company_doc.name, ) payroll_entry.create_salary_slips() payroll_entry.submit_salary_slips() @@ -419,41 +362,30 @@ class TestPayrollEntry(FrappeTestCase): self.assertIsNotNone(payroll_entry.error_message) # set accounts - for data in frappe.get_all("Salary Component", fields=["name"]): - if not frappe.db.get_value( - "Salary Component Account", {"parent": data.name, "company": company}, "name" - ): - set_salary_component_account(data.name, company_list=[company]) + for data in frappe.get_all("Salary Component", pluck="name"): + set_salary_component_account(data, company_list=[company]) # Payroll Entry successful, status should change to Submitted payroll_entry.submit_salary_slips() payroll_entry.reload() + self.assertEqual(payroll_entry.status, "Submitted") self.assertEqual(payroll_entry.error_message, "") def test_payroll_entry_status(self): - company = erpnext.get_default_company() - for data in frappe.get_all("Salary Component", fields=["name"]): - if not frappe.db.get_value( - "Salary Component Account", {"parent": data.name, "company": company}, "name" - ): - set_salary_component_account(data.name) - - employee = frappe.db.get_value("Employee", {"company": company}) + company = "_Test Company" company_doc = frappe.get_doc("Company", company) - make_salary_structure( - "_Test Salary Structure", - "Monthly", - employee, - company=company, - currency=company_doc.default_currency, - ) + employee = frappe.db.get_value("Employee", {"company": company}) + + setup_salary_structure(employee, company_doc) + dates = get_start_end_dates("Monthly", nowdate()) payroll_entry = get_payroll_entry_data( start_date=dates.start_date, end_date=dates.end_date, payable_account=company_doc.default_payroll_payable_account, currency=company_doc.default_currency, + company=company_doc.name, ) payroll_entry.submit() self.assertEqual(payroll_entry.status, "Submitted") @@ -532,3 +464,28 @@ def make_holiday(holiday_list_name): ).insert() return holiday_list_name + + +def get_salary_slip(user, period, salary_structure): + salary_slip = make_employee_salary_slip(user, period, salary_structure) + salary_slip.exchange_rate = 70 + salary_slip.calculate_net_pay() + salary_slip.db_update() + + return salary_slip + + +def setup_salary_structure(employee, company_doc, currency=None, salary_structure=None): + for data in frappe.get_all("Salary Component", pluck="name"): + if not frappe.db.get_value( + "Salary Component Account", {"parent": data, "company": company_doc.name}, "name" + ): + set_salary_component_account(data) + + make_salary_structure( + salary_structure or "_Test Salary Structure", + "Monthly", + employee, + company=company_doc.name, + currency=(currency or company_doc.default_currency), + ) From 661e05e6937fe75d9acc331dc4b18be5b16ec638 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 1 Jun 2022 17:28:42 +0530 Subject: [PATCH 08/16] fix(tests): account and company setups --- .../payroll_entry/test_payroll_entry.py | 55 ++++++------------- 1 file changed, 18 insertions(+), 37 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py index 47b9962912..84f1575381 100644 --- a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py @@ -86,44 +86,32 @@ class TestPayrollEntry(FrappeTestCase): ) def test_multi_currency_payroll_entry(self): - company = erpnext.get_default_company() - employee = make_employee("test_muti_currency_employee@payroll.com", company=company) - for data in frappe.get_all("Salary Component", fields=["name"]): - if not frappe.db.get_value( - "Salary Component Account", {"parent": data.name, "company": company}, "name" - ): - get_salary_component_account(data.name) + company = frappe.get_doc("Company", "_Test Company") + employee = make_employee( + "test_muti_currency_employee@payroll.com", company=company.name, department="Accounts - _TC" + ) + salary_structure = "_Test Multi Currency Salary Structure" + setup_salary_structure(employee, company, "USD", salary_structure) - company_doc = frappe.get_doc("Company", company) - salary_structure = make_salary_structure( - "_Test Multi Currency Salary Structure", "Monthly", company=company, currency="USD" - ) - create_salary_structure_assignment( - employee, salary_structure.name, company=company, currency="USD" - ) - frappe.db.sql( - """delete from `tabSalary Slip` where employee=%s""", - (frappe.db.get_value("Employee", {"user_id": "test_muti_currency_employee@payroll.com"})), - ) - salary_slip = get_salary_slip( - "test_muti_currency_employee@payroll.com", "Monthly", "_Test Multi Currency Salary Structure" - ) dates = get_start_end_dates("Monthly", nowdate()) payroll_entry = make_payroll_entry( start_date=dates.start_date, end_date=dates.end_date, - payable_account=company_doc.default_payroll_payable_account, + payable_account=company.default_payroll_payable_account, currency="USD", exchange_rate=70, + company=company.name, + cost_center="Main - _TC", ) payroll_entry.make_payment_entry() - salary_slip.load_from_db() + salary_slip = frappe.db.get_value("Salary Slip", {"payroll_entry": payroll_entry.name}, "name") + salary_slip = frappe.get_doc("Salary Slip", salary_slip) + payroll_entry.reload() payroll_je = salary_slip.journal_entry if payroll_je: payroll_je_doc = frappe.get_doc("Journal Entry", payroll_je) - self.assertEqual(salary_slip.base_gross_pay, payroll_je_doc.total_debit) self.assertEqual(salary_slip.base_gross_pay, payroll_je_doc.total_credit) @@ -136,7 +124,6 @@ class TestPayrollEntry(FrappeTestCase): (payroll_entry.name), as_dict=1, ) - self.assertEqual(salary_slip.base_net_pay, payment_entry[0].total_debit) self.assertEqual(salary_slip.base_net_pay, payment_entry[0].total_credit) @@ -306,7 +293,7 @@ class TestPayrollEntry(FrappeTestCase): def test_salary_slip_operation_queueing(self): company = "_Test Company" company_doc = frappe.get_doc("Company", company) - employee = frappe.db.get_value("Employee", {"company": company}) + employee = make_employee("test_employee@payroll.com", company=company) setup_salary_structure(employee, company_doc) # enqueue salary slip creation via payroll entry @@ -318,6 +305,7 @@ class TestPayrollEntry(FrappeTestCase): payable_account=company_doc.default_payroll_payable_account, currency=company_doc.default_currency, company=company_doc.name, + cost_center="Main - _TC", ) frappe.flags.enqueue_payroll_entry = True payroll_entry.create_salary_slips() @@ -329,7 +317,7 @@ class TestPayrollEntry(FrappeTestCase): def test_salary_slip_operation_failure(self): company = "_Test Company" company_doc = frappe.get_doc("Company", company) - employee = frappe.db.get_value("Employee", {"company": company}) + employee = make_employee("test_employee@payroll.com", company=company) salary_structure = make_salary_structure( "_Test Salary Structure", @@ -353,6 +341,7 @@ class TestPayrollEntry(FrappeTestCase): payable_account=company_doc.default_payroll_payable_account, currency=company_doc.default_currency, company=company_doc.name, + cost_center="Main - _TC", ) payroll_entry.create_salary_slips() payroll_entry.submit_salary_slips() @@ -375,7 +364,7 @@ class TestPayrollEntry(FrappeTestCase): def test_payroll_entry_status(self): company = "_Test Company" company_doc = frappe.get_doc("Company", company) - employee = frappe.db.get_value("Employee", {"company": company}) + employee = make_employee("test_employee@payroll.com", company=company) setup_salary_structure(employee, company_doc) @@ -386,6 +375,7 @@ class TestPayrollEntry(FrappeTestCase): payable_account=company_doc.default_payroll_payable_account, currency=company_doc.default_currency, company=company_doc.name, + cost_center="Main - _TC", ) payroll_entry.submit() self.assertEqual(payroll_entry.status, "Submitted") @@ -466,15 +456,6 @@ def make_holiday(holiday_list_name): return holiday_list_name -def get_salary_slip(user, period, salary_structure): - salary_slip = make_employee_salary_slip(user, period, salary_structure) - salary_slip.exchange_rate = 70 - salary_slip.calculate_net_pay() - salary_slip.db_update() - - return salary_slip - - def setup_salary_structure(employee, company_doc, currency=None, salary_structure=None): for data in frappe.get_all("Salary Component", pluck="name"): if not frappe.db.get_value( From c7efa3b44d033c5214fbf6453954b7c5de25e037 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 2 Jun 2022 12:27:11 +0530 Subject: [PATCH 09/16] ci: stale apt cache (#31217) --- .github/helper/install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/helper/install.sh b/.github/helper/install.sh index 69749c93af..f0f83b061b 100644 --- a/.github/helper/install.sh +++ b/.github/helper/install.sh @@ -11,7 +11,7 @@ fi cd ~ || exit -sudo apt-get install redis-server libcups2-dev +sudo apt update && sudo apt install redis-server libcups2-dev pip install frappe-bench From f0ac394d6e5d284a476eba62a6fcd6aaa01dd00d Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 2 Jun 2022 12:59:55 +0530 Subject: [PATCH 10/16] fix(India): GSTIN filter in GSTR-1 report --- erpnext/regional/report/gstr_1/gstr_1.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/erpnext/regional/report/gstr_1/gstr_1.py b/erpnext/regional/report/gstr_1/gstr_1.py index 0bdbe56de6..6cbc12c7a1 100644 --- a/erpnext/regional/report/gstr_1/gstr_1.py +++ b/erpnext/regional/report/gstr_1/gstr_1.py @@ -1155,8 +1155,11 @@ def get_company_gstins(company): .inner_join(links) .on(address.name == links.parent) .select(address.gstin) + .distinct() .where(links.link_doctype == "Company") .where(links.link_name == company) + .where(address.gstin.isnotnull()) + .where(address.gstin != "") .run(as_dict=1) ) From 3f376cc3a5b7f6f28957e032976d31287f7f88cb Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 2 Jun 2022 13:57:54 +0530 Subject: [PATCH 11/16] fix: Parent dimension filters in orders --- erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js | 2 -- erpnext/accounts/doctype/sales_invoice/sales_invoice.js | 1 - erpnext/buying/doctype/purchase_order/purchase_order.js | 2 -- erpnext/public/js/controllers/buying.js | 1 + erpnext/selling/sales_common.js | 1 + erpnext/stock/doctype/delivery_note/delivery_note.js | 2 -- erpnext/stock/doctype/purchase_receipt/purchase_receipt.js | 2 -- 7 files changed, 2 insertions(+), 9 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js index 42917f811d..7e3597e491 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js @@ -45,8 +45,6 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. if (this.frm.doc.supplier && this.frm.doc.__islocal) { this.frm.trigger('supplier'); } - - erpnext.accounts.dimensions.setup_dimension_filters(this.frm, this.frm.doctype); } refresh(doc) { diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js index 9dde85fe12..aefa9a59dd 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js @@ -52,7 +52,6 @@ erpnext.accounts.SalesInvoiceController = class SalesInvoiceController extends e me.frm.refresh_fields(); } erpnext.queries.setup_warehouse_query(this.frm); - erpnext.accounts.dimensions.setup_dimension_filters(this.frm, this.frm.doctype); } refresh(doc, dt, dn) { diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.js b/erpnext/buying/doctype/purchase_order/purchase_order.js index c9e67987c6..da45610eaf 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.js +++ b/erpnext/buying/doctype/purchase_order/purchase_order.js @@ -43,8 +43,6 @@ frappe.ui.form.on("Purchase Order", { erpnext.queries.setup_queries(frm, "Warehouse", function() { return erpnext.queries.warehouse(frm.doc); }); - - erpnext.accounts.dimensions.setup_dimension_filters(frm, frm.doctype); }, apply_tds: function(frm) { diff --git a/erpnext/public/js/controllers/buying.js b/erpnext/public/js/controllers/buying.js index 58eb891600..a5b7699040 100644 --- a/erpnext/public/js/controllers/buying.js +++ b/erpnext/public/js/controllers/buying.js @@ -74,6 +74,7 @@ erpnext.buying.BuyingController = class BuyingController extends erpnext.Transac me.frm.set_query('supplier_address', erpnext.queries.address_query); me.frm.set_query('billing_address', erpnext.queries.company_address_query); + erpnext.accounts.dimensions.setup_dimension_filters(me.frm, me.frm.doctype); if(this.frm.fields_dict.supplier) { this.frm.set_query("supplier", function() { diff --git a/erpnext/selling/sales_common.js b/erpnext/selling/sales_common.js index 0954de4e66..6cb53c3bbe 100644 --- a/erpnext/selling/sales_common.js +++ b/erpnext/selling/sales_common.js @@ -43,6 +43,7 @@ erpnext.selling.SellingController = class SellingController extends erpnext.Tran me.frm.set_query('shipping_address_name', erpnext.queries.address_query); me.frm.set_query('dispatch_address_name', erpnext.queries.dispatch_address_query); + erpnext.accounts.dimensions.setup_dimension_filters(me.frm, me.frm.doctype); if(this.frm.fields_dict.selling_price_list) { this.frm.set_query("selling_price_list", function() { diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.js b/erpnext/stock/doctype/delivery_note/delivery_note.js index 706ca36598..ea3cf1948b 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.js +++ b/erpnext/stock/doctype/delivery_note/delivery_note.js @@ -77,8 +77,6 @@ frappe.ui.form.on("Delivery Note", { } }); - erpnext.accounts.dimensions.setup_dimension_filters(frm, frm.doctype); - frm.set_df_property('packed_items', 'cannot_add_rows', true); frm.set_df_property('packed_items', 'cannot_delete_rows', true); }, diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js index 51ec598f72..754404b068 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js @@ -46,8 +46,6 @@ frappe.ui.form.on("Purchase Receipt", { erpnext.queries.setup_queries(frm, "Warehouse", function() { return erpnext.queries.warehouse(frm.doc); }); - - erpnext.accounts.dimensions.setup_dimension_filters(frm, frm.doctype); }, refresh: function(frm) { From d641f260352d5aff7ec77def19bb318dca4a5054 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 2 Jun 2022 15:08:49 +0530 Subject: [PATCH 12/16] fix: error handling and messages - remove savepoints since submission should stop if any error occurs - refactor variable naming and msgprints - test Salary Slip creation failure - fix(test): explicitly commit after payroll entry creation so that the first salary slip creation failure does not rollback the Payroll Entry insert --- .../doctype/payroll_entry/payroll_entry.py | 38 +++++++++---------- .../payroll_entry/test_payroll_entry.py | 28 ++++++++++---- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py index edcada1451..620fcadceb 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py @@ -54,7 +54,7 @@ class PayrollEntry(Document): if self.validate_employee_attendance(): frappe.throw(_("Cannot Submit, Employees left to mark attendance")) - def set_status(self, status=None, update=True): + def set_status(self, status=None, update=False): if not status: status = {0: "Draft", 1: "Submitted", 2: "Cancelled"}[self.docstatus or 0] @@ -850,7 +850,6 @@ def log_payroll_failure(process, payroll_entry, error): def create_salary_slips_for_employees(employees, args, publish_progress=True): try: - frappe.db.savepoint("salary_slip_creation") payroll_entry = frappe.get_doc("Payroll Entry", args.payroll_entry) salary_slips_exist_for = get_existing_salary_slips(employees, args) count = 0 @@ -879,7 +878,7 @@ def create_salary_slips_for_employees(employees, args, publish_progress=True): ) except Exception as e: - frappe.db.rollback(save_point="salary_slip_creation") + frappe.db.rollback() log_payroll_failure("creation", payroll_entry, e) finally: @@ -887,24 +886,25 @@ def create_salary_slips_for_employees(employees, args, publish_progress=True): frappe.publish_realtime("completed_salary_slip_creation") -def show_payroll_submission_status(submitted, not_submitted, salary_slip): - if not submitted and not not_submitted: +def show_payroll_submission_status(submitted, unsubmitted, payroll_entry): + if not submitted and not unsubmitted: frappe.msgprint( _( "No salary slip found to submit for the above selected criteria OR salary slip already submitted" ) ) - return - - if submitted: + elif submitted and not unsubmitted: frappe.msgprint( - _("Salary Slip submitted for period from {0} to {1}").format( - salary_slip.start_date, salary_slip.end_date + _("Salary Slips submitted for period from {0} to {1}").format( + payroll_entry.start_date, payroll_entry.end_date + ) + ) + elif unsubmitted: + frappe.msgprint( + _("Could not submit some Salary Slips: {}").format( + ", ".join(get_link_to_form("Salary Slip", entry) for entry in unsubmitted) ) ) - - if not_submitted: - frappe.msgprint(_("Could not submit some Salary Slips")) def get_existing_salary_slips(employees, args): @@ -922,23 +922,21 @@ def get_existing_salary_slips(employees, args): def submit_salary_slips_for_employees(payroll_entry, salary_slips, publish_progress=True): try: - frappe.db.savepoint("salary_slip_submission") - submitted = [] - not_submitted = [] + unsubmitted = [] frappe.flags.via_payroll_entry = True count = 0 for entry in salary_slips: salary_slip = frappe.get_doc("Salary Slip", entry[0]) if salary_slip.net_pay < 0: - not_submitted.append(entry[0]) + unsubmitted.append(entry[0]) else: try: salary_slip.submit() submitted.append(salary_slip) except frappe.ValidationError: - not_submitted.append(entry[0]) + unsubmitted.append(entry[0]) count += 1 if publish_progress: @@ -949,10 +947,10 @@ def submit_salary_slips_for_employees(payroll_entry, salary_slips, publish_progr payroll_entry.email_salary_slip(submitted) payroll_entry.db_set({"salary_slips_submitted": 1, "status": "Submitted", "error_message": ""}) - show_payroll_submission_status(submitted, not_submitted, salary_slip) + show_payroll_submission_status(submitted, unsubmitted, payroll_entry) except Exception as e: - frappe.db.rollback(save_point="salary_slip_submission") + frappe.db.rollback() log_payroll_failure("submission", payroll_entry, e) finally: diff --git a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py index 84f1575381..0363a0c3de 100644 --- a/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/test_payroll_entry.py @@ -299,7 +299,7 @@ class TestPayrollEntry(FrappeTestCase): # enqueue salary slip creation via payroll entry # Payroll Entry status should change to Queued dates = get_start_end_dates("Monthly", nowdate()) - payroll_entry = get_payroll_entry_data( + payroll_entry = get_payroll_entry( start_date=dates.start_date, end_date=dates.end_date, payable_account=company_doc.default_payroll_payable_account, @@ -308,7 +308,7 @@ class TestPayrollEntry(FrappeTestCase): cost_center="Main - _TC", ) frappe.flags.enqueue_payroll_entry = True - payroll_entry.create_salary_slips() + payroll_entry.submit() payroll_entry.reload() self.assertEqual(payroll_entry.status, "Queued") @@ -335,7 +335,7 @@ class TestPayrollEntry(FrappeTestCase): # salary slip submission via payroll entry # Payroll Entry status should change to Failed because of the missing account setup dates = get_start_end_dates("Monthly", nowdate()) - payroll_entry = get_payroll_entry_data( + payroll_entry = get_payroll_entry( start_date=dates.start_date, end_date=dates.end_date, payable_account=company_doc.default_payroll_payable_account, @@ -343,7 +343,16 @@ class TestPayrollEntry(FrappeTestCase): company=company_doc.name, cost_center="Main - _TC", ) - payroll_entry.create_salary_slips() + + # set employee as Inactive to check creation failure + frappe.db.set_value("Employee", employee, "status", "Inactive") + payroll_entry.submit() + payroll_entry.reload() + self.assertEqual(payroll_entry.status, "Failed") + self.assertIsNotNone(payroll_entry.error_message) + + frappe.db.set_value("Employee", employee, "status", "Active") + payroll_entry.submit() payroll_entry.submit_salary_slips() payroll_entry.reload() @@ -369,7 +378,7 @@ class TestPayrollEntry(FrappeTestCase): setup_salary_structure(employee, company_doc) dates = get_start_end_dates("Monthly", nowdate()) - payroll_entry = get_payroll_entry_data( + payroll_entry = get_payroll_entry( start_date=dates.start_date, end_date=dates.end_date, payable_account=company_doc.default_payroll_payable_account, @@ -384,7 +393,7 @@ class TestPayrollEntry(FrappeTestCase): self.assertEqual(payroll_entry.status, "Cancelled") -def get_payroll_entry_data(**args): +def get_payroll_entry(**args): args = frappe._dict(args) payroll_entry = frappe.new_doc("Payroll Entry") @@ -407,13 +416,16 @@ def get_payroll_entry_data(**args): payroll_entry.payment_account = args.payment_account payroll_entry.fill_employee_details() - payroll_entry.save() + payroll_entry.insert() + + # Commit so that the first salary slip creation failure does not rollback the Payroll Entry insert. + frappe.db.commit() # nosemgrep return payroll_entry def make_payroll_entry(**args): - payroll_entry = get_payroll_entry_data(**args) + payroll_entry = get_payroll_entry(**args) payroll_entry.submit() payroll_entry.submit_salary_slips() if payroll_entry.get_sal_slip_list(ss_status=1): From 1db4e623ab6d728ee71663a33a22023961f03ea0 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 2 Jun 2022 17:26:08 +0530 Subject: [PATCH 13/16] fix: payroll operations button visibility --- .../doctype/payroll_entry/payroll_entry.js | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js index a33f7665bd..b06f3502e2 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js @@ -40,27 +40,40 @@ frappe.ui.form.on('Payroll Entry', { }, refresh: function (frm) { - if (frm.doc.docstatus == 0) { - if (!frm.is_new()) { + if (frm.doc.docstatus === 0 && !frm.is_new()) { + frm.page.clear_primary_action(); + frm.add_custom_button(__("Get Employees"), + function () { + frm.events.get_employee_details(frm); + } + ).toggleClass("btn-primary", !(frm.doc.employees || []).length); + } + + if ( + (frm.doc.employees || []).length + && !frappe.model.has_workflow(frm.doctype) + && !cint(frm.doc.salary_slips_created) + && (frm.doc.docstatus != 2) + ) { + if (frm.doc.docstatus == 0) { frm.page.clear_primary_action(); - frm.add_custom_button(__("Get Employees"), - function () { - frm.events.get_employee_details(frm); - } - ).toggleClass('btn-primary', !(frm.doc.employees || []).length); - } - if ((frm.doc.employees || []).length && !frappe.model.has_workflow(frm.doctype)) { - frm.page.clear_primary_action(); - frm.page.set_primary_action(__('Create Salary Slips'), () => { - frm.save('Submit').then(() => { + frm.page.set_primary_action(__("Create Salary Slips"), () => { + frm.save("Submit").then(() => { frm.page.clear_primary_action(); frm.refresh(); frm.events.refresh(frm); }); }); + } else if (frm.doc.docstatus == 1 && frm.doc.status == "Failed") { + frm.add_custom_button(__("Create Salary Slip"), function () { + frm.call("create_salary_slips", {}, () => { + frm.reload_doc(); + }); + }).addClass("btn-primary"); } } - if (frm.doc.docstatus == 1) { + + if (frm.doc.docstatus == 1 && frm.doc.status == "Submitted") { if (frm.custom_buttons) frm.clear_custom_buttons(); frm.events.add_context_buttons(frm); } From 3a1c923e76c1f19e101e32d98f54f9ac7d9266bc Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Thu, 2 Jun 2022 14:15:50 -0400 Subject: [PATCH 14/16] fix: display currencies in validation message. --- erpnext/controllers/accounts_controller.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index bebfa6c76f..24a34d6313 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1465,8 +1465,10 @@ class AccountsController(TransactionBase): if not party_gle_currency and (party_account_currency != self.currency): frappe.throw( - _("Party Account {0} currency and document currency should be same").format( - frappe.bold(party_account) + _("Party Account {0} currency ({1}) and document currency ({2}) should be same").format( + frappe.bold(party_account), + party_account_currency, + self.currency ) ) From b061ea4cd2e66a9a0e2a96ef9174f7c0366c52e9 Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Thu, 2 Jun 2022 14:23:54 -0400 Subject: [PATCH 15/16] chore: linter --- erpnext/controllers/accounts_controller.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 24a34d6313..854c0d00f5 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1466,9 +1466,7 @@ class AccountsController(TransactionBase): if not party_gle_currency and (party_account_currency != self.currency): frappe.throw( _("Party Account {0} currency ({1}) and document currency ({2}) should be same").format( - frappe.bold(party_account), - party_account_currency, - self.currency + frappe.bold(party_account), party_account_currency, self.currency ) ) From db07831db781b66a0070212ef5a06a600638aa27 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sun, 5 Jun 2022 12:13:02 +0530 Subject: [PATCH 16/16] fix(India): Supplies from composite dealer not showing up --- erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py | 1 - 1 file changed, 1 deletion(-) diff --git a/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py b/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py index d6210abf80..91fccfa6e8 100644 --- a/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py +++ b/erpnext/regional/doctype/gstr_3b_report/gstr_3b_report.py @@ -148,7 +148,6 @@ class GSTR3BReport(Document): FROM `tabPurchase Invoice` p , `tabPurchase Invoice Item` i WHERE p.docstatus = 1 and p.name = i.parent and p.is_opening = 'No' - and p.gst_category != 'Registered Composition' and (i.is_nil_exempt = 1 or i.is_non_gst = 1 or p.gst_category = 'Registered Composition') and month(p.posting_date) = %s and year(p.posting_date) = %s and p.company = %s and p.company_gstin = %s