Merge branch 'version-13-hotfix' into e-commerce-refactor

.git-blame-ignore-revs

@@ -13,3 +13,4 @@
 
 # Whitespace trimming throughout codebase
 9bb69e711a5da43aaf8c8ecb5601aeffd89dbe5a
+f0bcb753fb7ebbb64bb0d6906d431d002f0f7d8f

.github/helper/.flake8_strict

@@ -0,0 +1,72 @@
+[flake8]
+ignore =
+    B007,
+    B950,
+    E101,
+    E111,
+    E114,
+    E116,
+    E117,
+    E121,
+    E122,
+    E123,
+    E124,
+    E125,
+    E126,
+    E127,
+    E128,
+    E131,
+    E201,
+    E202,
+    E203,
+    E211,
+    E221,
+    E222,
+    E223,
+    E224,
+    E225,
+    E226,
+    E228,
+    E231,
+    E241,
+    E242,
+    E251,
+    E261,
+    E262,
+    E265,
+    E266,
+    E271,
+    E272,
+    E273,
+    E274,
+    E301,
+    E302,
+    E303,
+    E305,
+    E306,
+    E401,
+    E402,
+    E501,
+    E502,
+    E701,
+    E702,
+    E703,
+    E741,
+    F401,
+    F403,
+    W191,
+    W291,
+    W292,
+    W293,
+    W391,
+    W503,
+    W504,
+    E711,
+    E129,
+    F841,
+    E713,
+    E712,
+
+
+max-line-length = 200
+exclude=.github/helper/semgrep_rules,test_*.py

.github/helper/semgrep_rules/report.py

@@ -0,0 +1,15 @@
+from frappe import _
+
+
+# ruleid: frappe-missing-translate-function-in-report-python
+{"label": "Field Label"}
+
+# ruleid: frappe-missing-translate-function-in-report-python
+dict(label="Field Label")
+
+
+# ok: frappe-missing-translate-function-in-report-python
+{"label": _("Field Label")}
+
+# ok: frappe-missing-translate-function-in-report-python
+dict(label=_("Field Label"))

.github/helper/semgrep_rules/report.yml

@@ -0,0 +1,21 @@
+rules:
+- id: frappe-missing-translate-function-in-report-python
+  paths:
+    include:
+    - "**/report"
+    exclude:
+    - "**/regional"
+  pattern-either:
+  - patterns:
+      - pattern: |
+          {..., "label": "...", ...}
+      - pattern-not: |
+          {..., "label": _("..."), ...}
+  - patterns:
+      - pattern: dict(..., label="...", ...)
+      - pattern-not: dict(..., label=_("..."), ...)
+  message: |
+      All user facing text must be wrapped in translate function. Please refer to translation documentation. https://frappeframework.com/docs/user/en/guides/basics/translations
+  languages: [python]
+  severity: ERROR
+

.github/workflows/linters.yml

@@ -1,11 +1,12 @@
-name: Semgrep
+name: Linters
 
 on:
   pull_request: { }
 
 jobs:
-  semgrep:
+
-    name: Frappe Linter
+  linters:
+    name: linters
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -16,3 +17,11 @@ jobs:
             config: >-
               r/python.lang.correctness
               .github/helper/semgrep_rules
+
+      - name: Set up Python 3.8
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.8
+
+      - name: Install and Run Pre-commit
+        uses: pre-commit/action@v2.0.0

.pre-commit-config.yaml

@@ -0,0 +1,29 @@
+exclude: 'node_modules|.git'
+default_stages: [commit]
+fail_fast: false
+
+
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.0.1
+    hooks:
+      - id: trailing-whitespace
+        files: "erpnext.*"
+        exclude: ".*json$|.*txt$|.*csv|.*md"
+      - id: check-yaml
+      - id: no-commit-to-branch
+        args: ['--branch', 'develop']
+      - id: check-merge-conflict
+      - id: check-ast
+
+  - repo: https://gitlab.com/pycqa/flake8
+    rev: 3.9.2
+    hooks:
+      - id: flake8
+        args: ['--config', '.github/helper/.flake8_strict']
+        exclude: ".*setup.py$"
+
+ci:
+    autoupdate_schedule: weekly
+    skip: []
+    submodules: false

cypress/integration/test_organizational_chart_desktop.js

@@ -2,7 +2,12 @@ context('Organizational Chart', () => {
 	before(() => {
 		cy.login();
 		cy.visit('/app/website');
+	});
+
+	it('navigates to org chart', () => {
+		cy.visit('/app');
 		cy.awesomebar('Organizational Chart');
+		cy.url().should('include', '/organizational-chart');
 
 		cy.window().its('frappe.csrf_token').then(csrf_token => {
 			return cy.request({

cypress/integration/test_organizational_chart_mobile.js

@@ -1,9 +1,14 @@
 context('Organizational Chart Mobile', () => {
 	before(() => {
 		cy.login();
-		cy.viewport(375, 667);
 		cy.visit('/app/website');
+	});
+
+	it('navigates to org chart', () => {
+		cy.viewport(375, 667);
+		cy.visit('/app');
 		cy.awesomebar('Organizational Chart');
+		cy.url().should('include', '/organizational-chart');
 
 		cy.window().its('frappe.csrf_token').then(csrf_token => {
 			return cy.request({

erpnext/accounts/deferred_revenue.py

@@ -359,7 +359,7 @@ def make_gl_entries(doc, credit_account, debit_account, against,
 		try:
 			make_gl_entries(gl_entries, cancel=(doc.docstatus == 2), merge_entries=True)
 			frappe.db.commit()
-		except:
+		except Exception:
 			frappe.db.rollback()
 			traceback = frappe.get_traceback()
 			frappe.log_error(message=traceback)
@@ -430,7 +430,7 @@ def book_revenue_via_journal_entry(doc, credit_account, debit_account, against,
 
 		if submit:
 			journal_entry.submit()
-	except:
+	except Exception:
 		frappe.db.rollback()
 		traceback = frappe.get_traceback()
 		frappe.log_error(message=traceback)

erpnext/accounts/doctype/payment_entry/payment_entry.js

@@ -872,7 +872,7 @@ frappe.ui.form.on('Payment Entry', {
 				&& frm.doc.base_total_allocated_amount < frm.doc.base_received_amount + total_deductions
 				&& frm.doc.total_allocated_amount < frm.doc.paid_amount + (total_deductions / frm.doc.source_exchange_rate)) {
 					unallocated_amount = (frm.doc.base_received_amount + total_deductions + frm.doc.base_total_taxes_and_charges
-						+ frm.doc.base_total_allocated_amount) / frm.doc.source_exchange_rate;
+						- frm.doc.base_total_allocated_amount) / frm.doc.source_exchange_rate;
 			} else if (frm.doc.payment_type == "Pay"
 				&& frm.doc.base_total_allocated_amount < frm.doc.base_paid_amount - total_deductions
 				&& frm.doc.total_allocated_amount < frm.doc.received_amount + (total_deductions / frm.doc.target_exchange_rate)) {

erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.js

@@ -2,46 +2,10 @@
 // For license information, please see license.txt
 
 frappe.provide("erpnext.accounts");
-
-frappe.ui.form.on("Payment Reconciliation Payment", {
-	invoice_number: function(frm, cdt, cdn) {
-		var row = locals[cdt][cdn];
-		if(row.invoice_number) {
-			var parts = row.invoice_number.split(' | ');
-			var invoice_type = parts[0];
-			var invoice_number = parts[1];
-
-			var invoice_amount = frm.doc.invoices.filter(function(d) {
-				return d.invoice_type === invoice_type && d.invoice_number === invoice_number;
-			})[0].outstanding_amount;
-
-			frappe.model.set_value(cdt, cdn, "allocated_amount", Math.min(invoice_amount, row.amount));
-
-			frm.call({
-				doc: frm.doc,
-				method: 'get_difference_amount',
-				args: {
-					child_row: row
-				},
-				callback: function(r, rt) {
-					if(r.message) {
-						frappe.model.set_value(cdt, cdn,
-							"difference_amount", r.message);
-					}
-				}
-			});
-		}
-	}
-});
-
 erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.extend({
 	onload: function() {
 		var me = this;
 
-		this.frm.set_query("party", function() {
-			check_mandatory(me.frm);
-		});
-
 		this.frm.set_query("party_type", function() {
 			return {
 				"filters": {
@@ -88,15 +52,36 @@ erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.ext
 
 	refresh: function() {
 		this.frm.disable_save();
-		this.toggle_primary_action();
+
+		if (this.frm.doc.receivable_payable_account) {
+			this.frm.add_custom_button(__('Get Unreconciled Entries'), () =>
+				this.frm.trigger("get_unreconciled_entries")
+			);
+		}
+		if (this.frm.doc.invoices.length && this.frm.doc.payments.length) {
+			this.frm.add_custom_button(__('Allocate'), () =>
+				this.frm.trigger("allocate")
+			);
+		}
+		if (this.frm.doc.allocation.length) {
+			this.frm.add_custom_button(__('Reconcile'), () =>
+				this.frm.trigger("reconcile")
+			);
+		}
 	},
 
-	onload_post_render: function() {
+	company: function() {
-		this.toggle_primary_action();
+		var me = this;
+		this.frm.set_value('receivable_payable_account', '');
+		me.frm.clear_table("allocation");
+		me.frm.clear_table("invoices");
+		me.frm.clear_table("payments");
+		me.frm.refresh_fields();
+		me.frm.trigger('party');
 	},
 
 	party: function() {
-		var me = this
+		var me = this;
 		if (!me.frm.doc.receivable_payable_account && me.frm.doc.party_type && me.frm.doc.party) {
 			return frappe.call({
 				method: "erpnext.accounts.party.get_party_account",
@@ -109,6 +94,7 @@ erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.ext
 					if (!r.exc && r.message) {
 						me.frm.set_value("receivable_payable_account", r.message);
 					}
+					me.frm.refresh();
 				}
 			});
 		}
@@ -120,16 +106,41 @@ erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.ext
 			doc: me.frm.doc,
 			method: 'get_unreconciled_entries',
 			callback: function(r, rt) {
-				me.set_invoice_options();
+				if (!(me.frm.doc.payments.length || me.frm.doc.invoices.length)) {
-				me.toggle_primary_action();
+					frappe.throw({message: __("No invoice and payment records found for this party")});
+				}
+				me.frm.refresh();
 			}
 		});
 
 	},
 
+	allocate: function() {
+		var me = this;
+		let payments = me.frm.fields_dict.payments.grid.get_selected_children();
+		if (!(payments.length)) {
+			payments = me.frm.doc.payments;
+		}
+		let invoices = me.frm.fields_dict.invoices.grid.get_selected_children();
+		if (!(invoices.length)) {
+			invoices = me.frm.doc.invoices;
+		}
+		return me.frm.call({
+			doc: me.frm.doc,
+			method: 'allocate_entries',
+			args: {
+				payments: payments,
+				invoices: invoices
+			},
+			callback: function() {
+				me.frm.refresh();
+			}
+		});
+	},
+
 	reconcile: function() {
 		var me = this;
-		var show_dialog = me.frm.doc.payments.filter(d => d.difference_amount && !d.difference_account);
+		var show_dialog = me.frm.doc.allocation.filter(d => d.difference_amount && !d.difference_account);
 
 		if (show_dialog && show_dialog.length) {
 
@@ -138,7 +149,7 @@ erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.ext
 				title: __("Select Difference Account"),
 				fields: [
 					{
-						fieldname: "payments", fieldtype: "Table", label: __("Payments"),
+						fieldname: "allocation", fieldtype: "Table", label: __("Allocation"),
 						data: this.data, in_place_edit: true,
 						get_data: () => {
 							return this.data;
@@ -179,10 +190,10 @@ erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.ext
 					},
 				],
 				primary_action: function() {
-					const args = dialog.get_values()["payments"];
+					const args = dialog.get_values()["allocation"];
 
 					args.forEach(d => {
-						frappe.model.set_value("Payment Reconciliation Payment", d.docname,
+						frappe.model.set_value("Payment Reconciliation Allocation", d.docname,
 							"difference_account", d.difference_account);
 					});
 
@@ -192,9 +203,9 @@ erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.ext
 				primary_action_label: __('Reconcile Entries')
 			});
 
-			this.frm.doc.payments.forEach(d => {
+			this.frm.doc.allocation.forEach(d => {
 				if (d.difference_amount && !d.difference_account) {
-					dialog.fields_dict.payments.df.data.push({
+					dialog.fields_dict.allocation.df.data.push({
 						'docname': d.name,
 						'reference_name': d.reference_name,
 						'difference_amount': d.difference_amount,
@@ -203,8 +214,8 @@ erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.ext
 				}
 			});
 
-			this.data = dialog.fields_dict.payments.df.data;
+			this.data = dialog.fields_dict.allocation.df.data;
-			dialog.fields_dict.payments.grid.refresh();
+			dialog.fields_dict.allocation.grid.refresh();
 			dialog.show();
 		} else {
 			this.reconcile_payment_entries();
@@ -218,48 +229,12 @@ erpnext.accounts.PaymentReconciliationController = frappe.ui.form.Controller.ext
 			doc: me.frm.doc,
 			method: 'reconcile',
 			callback: function(r, rt) {
-				me.set_invoice_options();
+				me.frm.clear_table("allocation");
-				me.toggle_primary_action();
+				me.frm.refresh_fields();
+				me.frm.refresh();
 			}
 		});
-	},
-
-	set_invoice_options: function() {
-		var me = this;
-		var invoices = [];
-
-		$.each(me.frm.doc.invoices || [], function(i, row) {
-			if (row.invoice_number && !in_list(invoices, row.invoice_number))
-				invoices.push(row.invoice_type + " | " + row.invoice_number);
-		});
-
-		if (invoices) {
-			this.frm.fields_dict.payments.grid.update_docfield_property(
-				'invoice_number', 'options', "\n" + invoices.join("\n")
-			);
-
-			$.each(me.frm.doc.payments || [], function(i, p) {
-				if(!in_list(invoices, cstr(p.invoice_number))) p.invoice_number = null;
-			});
-		}
-
-		refresh_field("payments");
-	},
-
-	toggle_primary_action: function() {
-		if ((this.frm.doc.payments || []).length) {
-			this.frm.fields_dict.reconcile.$input
-				&& this.frm.fields_dict.reconcile.$input.addClass("btn-primary");
-			this.frm.fields_dict.get_unreconciled_entries.$input
-				&& this.frm.fields_dict.get_unreconciled_entries.$input.removeClass("btn-primary");
-		} else {
-			this.frm.fields_dict.reconcile.$input
-				&& this.frm.fields_dict.reconcile.$input.removeClass("btn-primary");
-			this.frm.fields_dict.get_unreconciled_entries.$input
-				&& this.frm.fields_dict.get_unreconciled_entries.$input.addClass("btn-primary");
-		}
 	}
-
 });
 
 $.extend(cur_frm.cscript, new erpnext.accounts.PaymentReconciliationController({frm: cur_frm}));

erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.json

@@ -1,622 +1,206 @@
 {
+ "actions": [],
  "allow_copy": 1,
- "allow_events_in_timeline": 0, 
- "allow_guest_to_view": 0, 
- "allow_import": 0, 
- "allow_rename": 0, 
- "beta": 0, 
  "creation": "2014-07-09 12:04:51.681583",
- "custom": 0, 
- "docstatus": 0, 
  "doctype": "DocType",
- "document_type": "", 
- "editable_grid": 0, 
  "engine": "InnoDB",
+ "field_order": [
+  "company",
+  "party_type",
+  "column_break_4",
+  "party",
+  "receivable_payable_account",
+  "col_break1",
+  "from_invoice_date",
+  "to_invoice_date",
+  "minimum_invoice_amount",
+  "maximum_invoice_amount",
+  "invoice_limit",
+  "column_break_13",
+  "from_payment_date",
+  "to_payment_date",
+  "minimum_payment_amount",
+  "maximum_payment_amount",
+  "payment_limit",
+  "bank_cash_account",
+  "sec_break1",
+  "invoices",
+  "column_break_15",
+  "payments",
+  "sec_break2",
+  "allocation"
+ ],
  "fields": [
   {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "company",
    "fieldtype": "Link",
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
    "label": "Company",
-   "length": 0, 
-   "no_copy": 0, 
    "options": "Company",
-   "permlevel": 0, 
+   "reqd": 1
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 1, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "party_type",
    "fieldtype": "Link",
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
    "label": "Party Type",
-   "length": 0, 
-   "no_copy": 0, 
    "options": "DocType",
-   "permlevel": 0, 
+   "reqd": 1
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 1, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
+   "depends_on": "eval:doc.party_type",
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "depends_on": "", 
    "fieldname": "party",
    "fieldtype": "Dynamic Link",
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
    "label": "Party",
-   "length": 0, 
-   "no_copy": 0, 
    "options": "party_type",
-   "permlevel": 0, 
+   "reqd": 1
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 1, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
+   "depends_on": "eval:doc.company && doc.party",
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "receivable_payable_account",
    "fieldtype": "Link",
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
    "label": "Receivable / Payable Account",
-   "length": 0, 
-   "no_copy": 0, 
    "options": "Account",
-   "permlevel": 0, 
+   "reqd": 1
-   "precision": "", 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 1, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
+   "description": "This filter will be applied to Journal Entry.",
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "bank_cash_account",
    "fieldtype": "Link",
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
    "in_list_view": 1,
-   "in_standard_filter": 0, 
    "label": "Bank / Cash Account",
-   "length": 0, 
+   "options": "Account"
-   "no_copy": 0, 
-   "options": "Account", 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
+   "collapsible": 1,
-   "allow_in_quick_entry": 0, 
+   "collapsible_depends_on": "eval: doc.invoices.length == 0",
-   "allow_on_submit": 0, 
+   "depends_on": "eval:doc.receivable_payable_account",
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "col_break1",
-   "fieldtype": "Column Break", 
+   "fieldtype": "Section Break",
-   "hidden": 0, 
+   "label": "Filters"
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
+   "depends_on": "eval:(doc.payments).length || (doc.invoices).length",
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "from_date", 
-   "fieldtype": "Date", 
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 1, 
-   "in_standard_filter": 0, 
-   "label": "From Invoice Date", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
-  }, 
-  {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "to_date", 
-   "fieldtype": "Date", 
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 1, 
-   "in_standard_filter": 0, 
-   "label": "To Invoice Date", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
-  }, 
-  {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "minimum_amount", 
-   "fieldtype": "Currency", 
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "Minimum Invoice Amount", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
-  }, 
-  {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "maximum_amount", 
-   "fieldtype": "Currency", 
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "Maximum Invoice Amount", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
-  }, 
-  {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "description": "System will fetch all the entries if limit value is zero.", 
-   "fieldname": "limit", 
-   "fieldtype": "Int", 
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "Limit", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "precision": "", 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
-  }, 
-  {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "get_unreconciled_entries", 
-   "fieldtype": "Button", 
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "Get Unreconciled Entries", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
-  }, 
-  {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "sec_break1",
    "fieldtype": "Section Break",
-   "hidden": 0, 
+   "label": "Unreconciled Entries"
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "Unreconciled Payment Details", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "payments",
    "fieldtype": "Table",
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
    "label": "Payments",
-   "length": 0, 
+   "options": "Payment Reconciliation Payment"
-   "no_copy": 0, 
-   "options": "Payment Reconciliation Payment", 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
+   "depends_on": "allocation",
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
-   "fieldname": "reconcile", 
-   "fieldtype": "Button", 
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "Reconcile", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
-  }, 
-  {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "sec_break2",
    "fieldtype": "Section Break",
-   "hidden": 0, 
+   "label": "Allocated Entries"
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
-   "label": "Invoice/Journal Entry Details", 
-   "length": 0, 
-   "no_copy": 0, 
-   "permlevel": 0, 
-   "print_hide": 0, 
-   "print_hide_if_no_value": 0, 
-   "read_only": 0, 
-   "remember_last_selected_value": 0, 
-   "report_hide": 0, 
-   "reqd": 0, 
-   "search_index": 0, 
-   "set_only_once": 0, 
-   "translatable": 0, 
-   "unique": 0
   },
   {
-   "allow_bulk_edit": 0, 
-   "allow_in_quick_entry": 0, 
-   "allow_on_submit": 0, 
-   "bold": 0, 
-   "collapsible": 0, 
-   "columns": 0, 
    "fieldname": "invoices",
    "fieldtype": "Table",
-   "hidden": 0, 
-   "ignore_user_permissions": 0, 
-   "ignore_xss_filter": 0, 
-   "in_filter": 0, 
-   "in_global_search": 0, 
-   "in_list_view": 0, 
-   "in_standard_filter": 0, 
    "label": "Invoices",
-   "length": 0, 
+   "options": "Payment Reconciliation Invoice"
-   "no_copy": 0, 
+  },
-   "options": "Payment Reconciliation Invoice", 
+  {
-   "permlevel": 0, 
+   "fieldname": "column_break_15",
-   "print_hide": 0, 
+   "fieldtype": "Column Break"
-   "print_hide_if_no_value": 0, 
+  },
-   "read_only": 1, 
+  {
-   "remember_last_selected_value": 0, 
+   "fieldname": "allocation",
-   "report_hide": 0, 
+   "fieldtype": "Table",
-   "reqd": 0, 
+   "label": "Allocation",
-   "search_index": 0, 
+   "options": "Payment Reconciliation Allocation"
-   "set_only_once": 0, 
+  },
-   "translatable": 0, 
+  {
-   "unique": 0
+   "fieldname": "column_break_4",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "from_invoice_date",
+   "fieldtype": "Date",
+   "in_list_view": 1,
+   "label": "From Invoice Date"
+  },
+  {
+   "fieldname": "to_invoice_date",
+   "fieldtype": "Date",
+   "in_list_view": 1,
+   "label": "To Invoice Date"
+  },
+  {
+   "fieldname": "minimum_invoice_amount",
+   "fieldtype": "Currency",
+   "label": "Minimum Invoice Amount"
+  },
+  {
+   "description": "System will fetch all the entries if limit value is zero.",
+   "fieldname": "invoice_limit",
+   "fieldtype": "Int",
+   "label": "Invoice Limit"
+  },
+  {
+   "fieldname": "column_break_13",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "from_payment_date",
+   "fieldtype": "Date",
+   "label": "From Payment Date"
+  },
+  {
+   "fieldname": "to_payment_date",
+   "fieldtype": "Date",
+   "label": "To Payment Date"
+  },
+  {
+   "fieldname": "minimum_payment_amount",
+   "fieldtype": "Currency",
+   "label": "Minimum Payment Amount"
+  },
+  {
+   "fieldname": "maximum_payment_amount",
+   "fieldtype": "Currency",
+   "label": "Maximum Payment Amount"
+  },
+  {
+   "fieldname": "payment_limit",
+   "fieldtype": "Int",
+   "label": "Payment Limit"
+  },
+  {
+   "fieldname": "maximum_invoice_amount",
+   "fieldtype": "Currency",
+   "label": "Maximum Invoice Amount"
   }
  ],
- "has_web_view": 0, 
- "hide_heading": 0, 
  "hide_toolbar": 1,
  "icon": "icon-resize-horizontal",
- "idx": 0, 
- "image_view": 0, 
- "in_create": 0, 
- "is_submittable": 0, 
  "issingle": 1,
- "istable": 0, 
+ "links": [],
- "max_attachments": 0, 
+ "modified": "2021-08-30 13:05:51.977861",
- "menu_index": 0, 
- "modified": "2019-01-15 17:42:21.135214", 
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Payment Reconciliation",
- "name_case": "", 
  "owner": "Administrator",
  "permissions": [
   {
-   "amend": 0, 
-   "cancel": 0, 
    "create": 1,
    "delete": 1,
-   "email": 0, 
-   "export": 0, 
-   "if_owner": 0, 
-   "import": 0, 
-   "permlevel": 0, 
-   "print": 0, 
    "read": 1,
-   "report": 0, 
    "role": "Accounts Manager",
-   "set_user_permissions": 0, 
    "share": 1,
-   "submit": 0, 
    "write": 1
   },
   {
-   "amend": 0, 
-   "cancel": 0, 
    "create": 1,
    "delete": 1,
-   "email": 0, 
-   "export": 0, 
-   "if_owner": 0, 
-   "import": 0, 
-   "permlevel": 0, 
-   "print": 0, 
    "read": 1,
-   "report": 0, 
    "role": "Accounts User",
-   "set_user_permissions": 0, 
    "share": 1,
-   "submit": 0, 
    "write": 1
   }
  ],
- "quick_entry": 0, 
- "read_only": 0, 
- "read_only_onload": 0, 
- "show_name_in_global_search": 0, 
  "sort_field": "modified",
  "sort_order": "DESC",
- "track_changes": 1, 
+ "track_changes": 1
- "track_seen": 0, 
- "track_views": 0
 }

erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py

@@ -3,7 +3,7 @@
 
 from __future__ import unicode_literals
 import frappe, erpnext
-from frappe.utils import flt, today
+from frappe.utils import flt, today, getdate, nowdate
 from frappe import msgprint, _
 from frappe.model.document import Document
 from erpnext.accounts.utils import (get_outstanding_invoices,
@@ -27,24 +27,32 @@ class PaymentReconciliation(Document):
 		else:
 			dr_or_cr_notes = []
 
-		self.add_payment_entries(payment_entries + journal_entries + dr_or_cr_notes)
+		non_reconciled_payments = payment_entries + journal_entries + dr_or_cr_notes
+
+		if self.payment_limit:
+			non_reconciled_payments = non_reconciled_payments[:self.payment_limit]
+
+		non_reconciled_payments = sorted(non_reconciled_payments, key=lambda k: k['posting_date'] or getdate(nowdate()))
+
+		self.add_payment_entries(non_reconciled_payments)
 
 	def get_payment_entries(self):
 		order_doctype = "Sales Order" if self.party_type=="Customer" else "Purchase Order"
+		condition = self.get_conditions(get_payments=True)
 		payment_entries = get_advance_payment_entries(self.party_type, self.party,
-			self.receivable_payable_account, order_doctype, against_all_orders=True, limit=self.limit)
+			self.receivable_payable_account, order_doctype, against_all_orders=True, limit=self.payment_limit,
+			condition=condition)
 
 		return payment_entries
 
 	def get_jv_entries(self):
+		condition = self.get_conditions()
 		dr_or_cr = ("credit_in_account_currency" if erpnext.get_party_account_type(self.party_type) == 'Receivable'
 			else "debit_in_account_currency")
 
 		bank_account_condition = "t2.against_account like %(bank_cash_account)s" \
 				if self.bank_cash_account else "1=1"
 
-		limit_cond = "limit %s" % self.limit if self.limit else ""
-
 		journal_entries = frappe.db.sql("""
 			select
 				"Journal Entry" as reference_type, t1.name as reference_name,
@@ -56,7 +64,7 @@ class PaymentReconciliation(Document):
 			where
 				t1.name = t2.parent and t1.docstatus = 1 and t2.docstatus = 1
 				and t2.party_type = %(party_type)s and t2.party = %(party)s
-				and t2.account = %(account)s and {dr_or_cr} > 0
+				and t2.account = %(account)s and {dr_or_cr} > 0 {condition}
 				and (t2.reference_type is null or t2.reference_type = '' or
 					(t2.reference_type in ('Sales Order', 'Purchase Order')
 						and t2.reference_name is not null and t2.reference_name != ''))
@@ -65,11 +73,11 @@ class PaymentReconciliation(Document):
 					THEN 1=1
 					ELSE {bank_account_condition}
 				END)
-			order by t1.posting_date {limit_cond}
+			order by t1.posting_date
 			""".format(**{
 				"dr_or_cr": dr_or_cr,
 				"bank_account_condition": bank_account_condition,
-				"limit_cond": limit_cond
+				"condition": condition
 			}), {
 				"party_type": self.party_type,
 				"party": self.party,
@@ -80,6 +88,7 @@ class PaymentReconciliation(Document):
 		return list(journal_entries)
 
 	def get_dr_or_cr_notes(self):
+		condition = self.get_conditions(get_return_invoices=True)
 		dr_or_cr = ("credit_in_account_currency"
 			if erpnext.get_party_account_type(self.party_type) == 'Receivable' else "debit_in_account_currency")
 
@@ -90,7 +99,7 @@ class PaymentReconciliation(Document):
 			if self.party_type == 'Customer' else "Purchase Invoice")
 
 		return frappe.db.sql(""" SELECT doc.name as reference_name, %(voucher_type)s as reference_type,
-				(sum(gl.{dr_or_cr}) - sum(gl.{reconciled_dr_or_cr})) as amount,
+				(sum(gl.{dr_or_cr}) - sum(gl.{reconciled_dr_or_cr})) as amount, doc.posting_date,
 				account_currency as currency
 			FROM `tab{doc}` doc, `tabGL Entry` gl
 			WHERE
@@ -100,15 +109,17 @@ class PaymentReconciliation(Document):
 				and gl.against_voucher_type = %(voucher_type)s
 				and doc.docstatus = 1 and gl.party = %(party)s
 				and gl.party_type = %(party_type)s and gl.account = %(account)s
-				and gl.is_cancelled = 0
+				and gl.is_cancelled = 0 {condition}
 			GROUP BY doc.name
 			Having
 				amount > 0
+			ORDER BY doc.posting_date
 		""".format(
 			doc=voucher_type,
 			dr_or_cr=dr_or_cr,
 			reconciled_dr_or_cr=reconciled_dr_or_cr,
-			party_type_field=frappe.scrub(self.party_type)),
+			party_type_field=frappe.scrub(self.party_type),
+			condition=condition or ""),
 			{
 				'party': self.party,
 				'party_type': self.party_type,
@@ -116,22 +127,23 @@ class PaymentReconciliation(Document):
 				'account': self.receivable_payable_account
 			}, as_dict=1)
 
-	def add_payment_entries(self, entries):
+	def add_payment_entries(self, non_reconciled_payments):
 		self.set('payments', [])
-		for e in entries:
+
+		for payment in non_reconciled_payments:
 			row = self.append('payments', {})
-			row.update(e)
+			row.update(payment)
 
 	def get_invoice_entries(self):
 		#Fetch JVs, Sales and Purchase Invoices for 'invoices' to reconcile against
 
-		condition = self.check_condition()
+		condition = self.get_conditions(get_invoices=True)
 
 		non_reconciled_invoices = get_outstanding_invoices(self.party_type, self.party,
 			self.receivable_payable_account, condition=condition)
 
-		if self.limit:
+		if self.invoice_limit:
-			non_reconciled_invoices = non_reconciled_invoices[:self.limit]
+			non_reconciled_invoices = non_reconciled_invoices[:self.invoice_limit]
 
 		self.add_invoice_entries(non_reconciled_invoices)
 
@@ -139,41 +151,78 @@ class PaymentReconciliation(Document):
 		#Populate 'invoices' with JVs and Invoices to reconcile against
 		self.set('invoices', [])
 
-		for e in non_reconciled_invoices:
+		for entry in non_reconciled_invoices:
-			ent = self.append('invoices', {})
+			inv = self.append('invoices', {})
-			ent.invoice_type = e.get('voucher_type')
+			inv.invoice_type = entry.get('voucher_type')
-			ent.invoice_number = e.get('voucher_no')
+			inv.invoice_number = entry.get('voucher_no')
-			ent.invoice_date = e.get('posting_date')
+			inv.invoice_date = entry.get('posting_date')
-			ent.amount = flt(e.get('invoice_amount'))
+			inv.amount = flt(entry.get('invoice_amount'))
-			ent.currency = e.get('currency')
+			inv.currency = entry.get('currency')
-			ent.outstanding_amount = e.get('outstanding_amount')
+			inv.outstanding_amount = flt(entry.get('outstanding_amount'))
 
 	@frappe.whitelist()
-	def reconcile(self, args):
+	def allocate_entries(self, args):
-		for e in self.get('payments'):
+		self.validate_entries()
-			e.invoice_type = None
+		entries = []
-			if e.invoice_number and " | " in e.invoice_number:
+		for pay in args.get('payments'):
-				e.invoice_type, e.invoice_number = e.invoice_number.split(" | ")
+			pay.update({'unreconciled_amount': pay.get('amount')})
+			for inv in args.get('invoices'):
+				if pay.get('amount') >= inv.get('outstanding_amount'):
+					res = self.get_allocated_entry(pay, inv, inv['outstanding_amount'])
+					pay['amount'] = flt(pay.get('amount')) - flt(inv.get('outstanding_amount'))
+					inv['outstanding_amount'] = 0
+				else:
+					res = self.get_allocated_entry(pay, inv, pay['amount'])
+					inv['outstanding_amount'] = flt(inv.get('outstanding_amount')) - flt(pay.get('amount'))
+					pay['amount'] = 0
+				if pay.get('amount') == 0:
+					entries.append(res)
+					break
+				elif inv.get('outstanding_amount') == 0:
+					entries.append(res)
+					continue
+			else:
+				break
 
-		self.get_invoice_entries()
+		self.set('allocation', [])
-		self.validate_invoice()
+		for entry in entries:
+			if entry['allocated_amount'] != 0:
+				row = self.append('allocation', {})
+				row.update(entry)
+
+	def get_allocated_entry(self, pay, inv, allocated_amount):
+		return frappe._dict({
+			'reference_type': pay.get('reference_type'),
+			'reference_name': pay.get('reference_name'),
+			'reference_row': pay.get('reference_row'),
+			'invoice_type': inv.get('invoice_type'),
+			'invoice_number': inv.get('invoice_number'),
+			'unreconciled_amount': pay.get('unreconciled_amount'),
+			'amount': pay.get('amount'),
+			'allocated_amount': allocated_amount,
+			'difference_amount': pay.get('difference_amount')
+		})
+
+	@frappe.whitelist()
+	def reconcile(self):
+		self.validate_allocation()
 		dr_or_cr = ("credit_in_account_currency"
 			if erpnext.get_party_account_type(self.party_type) == 'Receivable' else "debit_in_account_currency")
 
-		lst = []
+		entry_list = []
 		dr_or_cr_notes = []
-		for e in self.get('payments'):
+		for row in self.get('allocation'):
 			reconciled_entry = []
-			if e.invoice_number and e.allocated_amount:
+			if row.invoice_number and row.allocated_amount:
-				if e.reference_type in ['Sales Invoice', 'Purchase Invoice']:
+				if row.reference_type in ['Sales Invoice', 'Purchase Invoice']:
 					reconciled_entry = dr_or_cr_notes
 				else:
-					reconciled_entry = lst
+					reconciled_entry = entry_list
 
-				reconciled_entry.append(self.get_payment_details(e, dr_or_cr))
+				reconciled_entry.append(self.get_payment_details(row, dr_or_cr))
 
-		if lst:
+		if entry_list:
-			reconcile_against_document(lst)
+			reconcile_against_document(entry_list)
 
 		if dr_or_cr_notes:
 			reconcile_dr_cr_note(dr_or_cr_notes, self.company)
@@ -183,98 +232,104 @@ class PaymentReconciliation(Document):
 
 	def get_payment_details(self, row, dr_or_cr):
 		return frappe._dict({
-			'voucher_type': row.reference_type,
+			'voucher_type': row.get('reference_type'),
-			'voucher_no' : row.reference_name,
+			'voucher_no' : row.get('reference_name'),
-			'voucher_detail_no' : row.reference_row,
+			'voucher_detail_no' : row.get('reference_row'),
-			'against_voucher_type' : row.invoice_type,
+			'against_voucher_type' : row.get('invoice_type'),
-			'against_voucher'  : row.invoice_number,
+			'against_voucher'  : row.get('invoice_number'),
 			'account' : self.receivable_payable_account,
 			'party_type': self.party_type,
 			'party': self.party,
-			'is_advance' : row.is_advance,
+			'is_advance' : row.get('is_advance'),
 			'dr_or_cr' : dr_or_cr,
-			'unadjusted_amount' : flt(row.amount),
+			'unreconciled_amount': flt(row.get('unreconciled_amount')),
-			'allocated_amount' : flt(row.allocated_amount),
+			'unadjusted_amount' : flt(row.get('amount')),
-			'difference_amount': row.difference_amount,
+			'allocated_amount' : flt(row.get('allocated_amount')),
-			'difference_account': row.difference_account
+			'difference_amount': flt(row.get('difference_amount')),
+			'difference_account': row.get('difference_account')
 		})
 
-	@frappe.whitelist()
-	def get_difference_amount(self, child_row):
-		if child_row.get("reference_type") != 'Payment Entry': return
-
-		child_row = frappe._dict(child_row)
-
-		if child_row.invoice_number and " | " in child_row.invoice_number:
-			child_row.invoice_type, child_row.invoice_number = child_row.invoice_number.split(" | ")
-
-		dr_or_cr = ("credit_in_account_currency"
-			if erpnext.get_party_account_type(self.party_type) == 'Receivable' else "debit_in_account_currency")
-
-		row = self.get_payment_details(child_row, dr_or_cr)
-
-		doc = frappe.get_doc(row.voucher_type, row.voucher_no)
-		update_reference_in_payment_entry(row, doc, do_not_save=True)
-
-		return doc.difference_amount
-
 	def check_mandatory_to_fetch(self):
 		for fieldname in ["company", "party_type", "party", "receivable_payable_account"]:
 			if not self.get(fieldname):
 				frappe.throw(_("Please select {0} first").format(self.meta.get_label(fieldname)))
 
-	def validate_invoice(self):
+	def validate_entries(self):
 		if not self.get("invoices"):
-			frappe.throw(_("No records found in the Invoice table"))
+			frappe.throw(_("No records found in the Invoices table"))
 
 		if not self.get("payments"):
-			frappe.throw(_("No records found in the Payment table"))
+			frappe.throw(_("No records found in the Payments table"))
 
+	def validate_allocation(self):
 		unreconciled_invoices = frappe._dict()
-		for d in self.get("invoices"):
+
-			unreconciled_invoices.setdefault(d.invoice_type, {}).setdefault(d.invoice_number, d.outstanding_amount)
+		for inv in self.get("invoices"):
+			unreconciled_invoices.setdefault(inv.invoice_type, {}).setdefault(inv.invoice_number, inv.outstanding_amount)
 
 		invoices_to_reconcile = []
-		for p in self.get("payments"):
+		for row in self.get("allocation"):
-			if p.invoice_type and p.invoice_number and p.allocated_amount:
+			if row.invoice_type and row.invoice_number and row.allocated_amount:
-				invoices_to_reconcile.append(p.invoice_number)
+				invoices_to_reconcile.append(row.invoice_number)
 
-				if p.invoice_number not in unreconciled_invoices.get(p.invoice_type, {}):
+				if flt(row.amount) - flt(row.allocated_amount) < 0:
-					frappe.throw(_("{0}: {1} not found in Invoice Details table")
+					frappe.throw(_("Row {0}: Allocated amount {1} must be less than or equal to remaining payment amount {2}")
-						.format(p.invoice_type, p.invoice_number))
+						.format(row.idx, row.allocated_amount, row.amount))
 
-				if flt(p.allocated_amount) > flt(p.amount):
+				invoice_outstanding = unreconciled_invoices.get(row.invoice_type, {}).get(row.invoice_number)
-					frappe.throw(_("Row {0}: Allocated amount {1} must be less than or equals to Payment Entry amount {2}")
+				if flt(row.allocated_amount) - invoice_outstanding > 0.009:
-						.format(p.idx, p.allocated_amount, p.amount))
+					frappe.throw(_("Row {0}: Allocated amount {1} must be less than or equal to invoice outstanding amount {2}")
-
+						.format(row.idx, row.allocated_amount, invoice_outstanding))
-				invoice_outstanding = unreconciled_invoices.get(p.invoice_type, {}).get(p.invoice_number)
-				if flt(p.allocated_amount) - invoice_outstanding > 0.009:
-					frappe.throw(_("Row {0}: Allocated amount {1} must be less than or equals to invoice outstanding amount {2}")
-						.format(p.idx, p.allocated_amount, invoice_outstanding))
 
 		if not invoices_to_reconcile:
-			frappe.throw(_("Please select Allocated Amount, Invoice Type and Invoice Number in atleast one row"))
+			frappe.throw(_("No records found in Allocation table"))
 
-	def check_condition(self):
+	def get_conditions(self, get_invoices=False, get_payments=False, get_return_invoices=False):
-		cond = " and posting_date >= {0}".format(frappe.db.escape(self.from_date)) if self.from_date else ""
+		condition = " and company = '{0}' ".format(self.company)
-		cond += " and posting_date <= {0}".format(frappe.db.escape(self.to_date)) if self.to_date else ""
-		dr_or_cr = ("debit_in_account_currency" if erpnext.get_party_account_type(self.party_type) == 'Receivable'
-			else "credit_in_account_currency")
 
-		if self.minimum_amount:
+		if get_invoices:
-			cond += " and `{0}` >= {1}".format(dr_or_cr, flt(self.minimum_amount))
+			condition += " and posting_date >= {0}".format(frappe.db.escape(self.from_invoice_date)) if self.from_invoice_date else ""
-		if self.maximum_amount:
+			condition += " and posting_date <= {0}".format(frappe.db.escape(self.to_invoice_date)) if self.to_invoice_date else ""
-			cond += " and `{0}` <= {1}".format(dr_or_cr, flt(self.maximum_amount))
+			dr_or_cr = ("debit_in_account_currency" if erpnext.get_party_account_type(self.party_type) == 'Receivable'
+				else "credit_in_account_currency")
 
-		return cond
+			if self.minimum_invoice_amount:
+				condition += " and `{0}` >= {1}".format(dr_or_cr, flt(self.minimum_invoice_amount))
+			if self.maximum_invoice_amount:
+				condition += " and `{0}` <= {1}".format(dr_or_cr, flt(self.maximum_invoice_amount))
+
+		elif get_return_invoices:
+			condition = " and doc.company = '{0}' ".format(self.company)
+			condition += " and doc.posting_date >= {0}".format(frappe.db.escape(self.from_payment_date)) if self.from_payment_date else ""
+			condition += " and doc.posting_date <= {0}".format(frappe.db.escape(self.to_payment_date)) if self.to_payment_date else ""
+			dr_or_cr = ("gl.debit_in_account_currency" if erpnext.get_party_account_type(self.party_type) == 'Receivable'
+				else "gl.credit_in_account_currency")
+
+			if self.minimum_invoice_amount:
+				condition += " and `{0}` >= {1}".format(dr_or_cr, flt(self.minimum_payment_amount))
+			if self.maximum_invoice_amount:
+				condition += " and `{0}` <= {1}".format(dr_or_cr, flt(self.maximum_payment_amount))
+
+		else:
+			condition += " and posting_date >= {0}".format(frappe.db.escape(self.from_payment_date)) if self.from_payment_date else ""
+			condition += " and posting_date <= {0}".format(frappe.db.escape(self.to_payment_date)) if self.to_payment_date else ""
+
+			if self.minimum_payment_amount:
+				condition += " and unallocated_amount >= {0}".format(flt(self.minimum_payment_amount)) if get_payments \
+					else " and total_debit >= {0}".format(flt(self.minimum_payment_amount))
+			if self.maximum_payment_amount:
+				condition += " and unallocated_amount <= {0}".format(flt(self.maximum_payment_amount)) if get_payments \
+					else " and total_debit <= {0}".format(flt(self.maximum_payment_amount))
+
+		return condition
 
 def reconcile_dr_cr_note(dr_cr_notes, company):
-	for d in dr_cr_notes:
+	for inv in dr_cr_notes:
 		voucher_type = ('Credit Note'
-			if d.voucher_type == 'Sales Invoice' else 'Debit Note')
+			if inv.voucher_type == 'Sales Invoice' else 'Debit Note')
 
 		reconcile_dr_or_cr = ('debit_in_account_currency'
-			if d.dr_or_cr == 'credit_in_account_currency' else 'credit_in_account_currency')
+			if inv.dr_or_cr == 'credit_in_account_currency' else 'credit_in_account_currency')
 
 		company_currency = erpnext.get_company_currency(company)
 
@@ -283,25 +338,25 @@ def reconcile_dr_cr_note(dr_cr_notes, company):
 			"voucher_type": voucher_type,
 			"posting_date": today(),
 			"company": company,
-			"multi_currency": 1 if d.currency != company_currency else 0,
+			"multi_currency": 1 if inv.currency != company_currency else 0,
 			"accounts": [
 				{
-					'account': d.account,
+					'account': inv.account,
-					'party': d.party,
+					'party': inv.party,
-					'party_type': d.party_type,
+					'party_type': inv.party_type,
-					d.dr_or_cr: abs(d.allocated_amount),
+					inv.dr_or_cr: abs(inv.allocated_amount),
-					'reference_type': d.against_voucher_type,
+					'reference_type': inv.against_voucher_type,
-					'reference_name': d.against_voucher,
+					'reference_name': inv.against_voucher,
 					'cost_center': erpnext.get_default_cost_center(company)
 				},
 				{
-					'account': d.account,
+					'account': inv.account,
-					'party': d.party,
+					'party': inv.party,
-					'party_type': d.party_type,
+					'party_type': inv.party_type,
-					reconcile_dr_or_cr: (abs(d.allocated_amount)
+					reconcile_dr_or_cr: (abs(inv.allocated_amount)
-						if abs(d.unadjusted_amount) > abs(d.allocated_amount) else abs(d.unadjusted_amount)),
+						if abs(inv.unadjusted_amount) > abs(inv.allocated_amount) else abs(inv.unadjusted_amount)),
-					'reference_type': d.voucher_type,
+					'reference_type': inv.voucher_type,
-					'reference_name': d.voucher_no,
+					'reference_name': inv.voucher_no,
 					'cost_center': erpnext.get_default_cost_center(company)
 				}
 			]

erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py

@@ -0,0 +1,8 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# See license.txt
+
+# import frappe
+import unittest
+
+class TestPaymentReconciliation(unittest.TestCase):
+	pass

erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.json

@@ -0,0 +1,137 @@
+{
+ "actions": [],
+ "creation": "2021-08-16 17:04:40.185167",
+ "doctype": "DocType",
+ "editable_grid": 1,
+ "engine": "InnoDB",
+ "field_order": [
+  "reference_type",
+  "reference_name",
+  "column_break_3",
+  "invoice_type",
+  "invoice_number",
+  "section_break_6",
+  "allocated_amount",
+  "unreconciled_amount",
+  "amount",
+  "column_break_8",
+  "is_advance",
+  "section_break_5",
+  "difference_amount",
+  "column_break_7",
+  "difference_account"
+ ],
+ "fields": [
+  {
+   "fieldname": "invoice_number",
+   "fieldtype": "Dynamic Link",
+   "in_list_view": 1,
+   "label": "Invoice Number",
+   "options": "invoice_type",
+   "read_only": 1,
+   "reqd": 1
+  },
+  {
+   "fieldname": "allocated_amount",
+   "fieldtype": "Currency",
+   "in_list_view": 1,
+   "label": "Allocated Amount",
+   "options": "Currency",
+   "reqd": 1
+  },
+  {
+   "fieldname": "column_break_3",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "section_break_5",
+   "fieldtype": "Section Break"
+  },
+  {
+   "fieldname": "difference_account",
+   "fieldtype": "Link",
+   "label": "Difference Account",
+   "options": "Account",
+   "read_only": 1
+  },
+  {
+   "fieldname": "column_break_7",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "difference_amount",
+   "fieldtype": "Currency",
+   "in_list_view": 1,
+   "label": "Difference Amount",
+   "options": "Currency",
+   "read_only": 1
+  },
+  {
+   "fieldname": "reference_name",
+   "fieldtype": "Dynamic Link",
+   "in_list_view": 1,
+   "label": "Reference Name",
+   "options": "reference_type",
+   "read_only": 1,
+   "reqd": 1
+  },
+  {
+   "fieldname": "is_advance",
+   "fieldtype": "Data",
+   "hidden": 1,
+   "label": "Is Advance",
+   "read_only": 1
+  },
+  {
+   "fieldname": "reference_type",
+   "fieldtype": "Link",
+   "label": "Reference Type",
+   "options": "DocType",
+   "read_only": 1,
+   "reqd": 1
+  },
+  {
+   "fieldname": "invoice_type",
+   "fieldtype": "Link",
+   "label": "Invoice Type",
+   "options": "DocType",
+   "read_only": 1,
+   "reqd": 1
+  },
+  {
+   "fieldname": "section_break_6",
+   "fieldtype": "Section Break"
+  },
+  {
+   "fieldname": "column_break_8",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "unreconciled_amount",
+   "fieldtype": "Currency",
+   "hidden": 1,
+   "label": "Unreconciled Amount",
+   "options": "Currency",
+   "read_only": 1
+  },
+  {
+   "fieldname": "amount",
+   "fieldtype": "Currency",
+   "hidden": 1,
+   "label": "Amount",
+   "options": "Currency",
+   "read_only": 1
+  }
+ ],
+ "istable": 1,
+ "links": [],
+ "modified": "2021-08-30 10:58:42.665107",
+ "modified_by": "Administrator",
+ "module": "Accounts",
+ "name": "Payment Reconciliation Allocation",
+ "owner": "Administrator",
+ "permissions": [],
+ "sort_field": "modified",
+ "sort_order": "DESC",
+ "track_changes": 1
+}

erpnext/accounts/doctype/payment_reconciliation_allocation/payment_reconciliation_allocation.py

@@ -0,0 +1,8 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+# import frappe
+from frappe.model.document import Document
+
+class PaymentReconciliationAllocation(Document):
+	pass

erpnext/accounts/doctype/payment_reconciliation_invoice/payment_reconciliation_invoice.json

@@ -44,7 +44,6 @@
   {
    "fieldname": "amount",
    "fieldtype": "Currency",
-   "in_list_view": 1,
    "label": "Amount",
    "options": "currency",
    "read_only": 1
@@ -67,7 +66,7 @@
  ],
  "istable": 1,
  "links": [],
- "modified": "2020-07-19 18:12:27.964073",
+ "modified": "2021-08-24 22:42:40.923179",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Payment Reconciliation Invoice",

erpnext/accounts/doctype/payment_reconciliation_payment/payment_reconciliation_payment.json

@@ -11,11 +11,7 @@
   "is_advance",
   "reference_row",
   "col_break1",
-  "invoice_number",
   "amount",
-  "allocated_amount",
-  "section_break_10",
-  "difference_account",
   "difference_amount",
   "sec_break1",
   "remark",
@@ -41,6 +37,7 @@
   {
    "fieldname": "posting_date",
    "fieldtype": "Date",
+   "in_list_view": 1,
    "label": "Posting Date",
    "read_only": 1
   },
@@ -62,14 +59,6 @@
    "fieldname": "col_break1",
    "fieldtype": "Column Break"
   },
-  {
-   "columns": 2,
-   "fieldname": "invoice_number",
-   "fieldtype": "Select",
-   "in_list_view": 1,
-   "label": "Invoice Number",
-   "reqd": 1
-  },
   {
    "columns": 2,
    "fieldname": "amount",
@@ -79,15 +68,6 @@
    "options": "currency",
    "read_only": 1
   },
-  {
-   "columns": 2,
-   "fieldname": "allocated_amount",
-   "fieldtype": "Currency",
-   "in_list_view": 1,
-   "label": "Allocated amount",
-   "options": "currency",
-   "reqd": 1
-  },
   {
    "fieldname": "sec_break1",
    "fieldtype": "Section Break"
@@ -95,41 +75,27 @@
   {
    "fieldname": "remark",
    "fieldtype": "Small Text",
-   "in_list_view": 1,
    "label": "Remark",
    "read_only": 1
   },
-  {
-   "columns": 2,
-   "fieldname": "difference_account",
-   "fieldtype": "Link",
-   "in_list_view": 1,
-   "label": "Difference Account",
-   "options": "Account"
-  },
-  {
-   "fieldname": "difference_amount",
-   "fieldtype": "Currency",
-   "label": "Difference Amount",
-   "options": "currency",
-   "print_hide": 1,
-   "read_only": 1
-  },
-  {
-   "fieldname": "section_break_10",
-   "fieldtype": "Section Break"
-  },
   {
    "fieldname": "currency",
    "fieldtype": "Link",
    "hidden": 1,
    "label": "Currency",
    "options": "Currency"
+  },
+  {
+   "fieldname": "difference_amount",
+   "fieldtype": "Currency",
+   "label": "Difference Amount",
+   "options": "currency",
+   "read_only": 1
   }
  ],
  "istable": 1,
  "links": [],
- "modified": "2020-07-19 18:12:41.682347",
+ "modified": "2021-08-30 10:51:48.140062",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Payment Reconciliation Payment",

erpnext/accounts/doctype/pricing_rule/pricing_rule.py

@@ -315,9 +315,8 @@ def update_args_for_pricing_rule(args):
 	if not (args.item_group and args.brand):
 		try:
 			args.item_group, args.brand = frappe.get_cached_value("Item", args.item_code, ["item_group", "brand"])
-		except TypeError:
+		except frappe.DoesNotExistError:
-			# invalid item_code
+			return
-			return item_details
 		if not args.item_group:
 			frappe.throw(_("Item Group not mentioned in item master for item {0}").format(args.item_code))
 

erpnext/accounts/doctype/pricing_rule/utils.py

@@ -81,7 +81,7 @@ def filter_pricing_rule_based_on_condition(pricing_rules, doc=None):
 				try:
 					if frappe.safe_eval(pricing_rule.condition, None, doc.as_dict()):
 						filtered_pricing_rules.append(pricing_rule)
-				except:
+				except Exception:
 					pass
 			else:
 				filtered_pricing_rules.append(pricing_rule)

erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py

@@ -158,7 +158,7 @@ def get_recipients_and_cc(customer, doc):
 	if doc.cc_to != '':
 		try:
 			cc=[frappe.get_value('User', doc.cc_to, 'email')]
-		except:
+		except Exception:
 			pass
 
 	return recipients, cc

erpnext/accounts/doctype/sales_invoice/sales_invoice.py

@@ -24,7 +24,6 @@ from erpnext.accounts.deferred_revenue import validate_service_stop_date
 from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category import get_party_tax_withholding_details
 from frappe.model.utils import get_fetch_values
 from frappe.contacts.doctype.address.address import get_address_display
-from erpnext.accounts.doctype.tax_withholding_category.tax_withholding_category import get_party_tax_withholding_details
 
 from erpnext.healthcare.utils import manage_invoice_submit_cancel
 

erpnext/accounts/party.py

@@ -648,7 +648,7 @@ def get_default_contact(doctype, name):
 	if out:
 		try:
 			return out[0][0]
-		except:
+		except Exception:
 			return None
 	else:
 		return None

erpnext/accounts/report/financial_statements.py

@@ -339,7 +339,7 @@ def sort_accounts(accounts, is_root=False, key="name"):
 	"""Sort root types as Asset, Liability, Equity, Income, Expense"""
 
 	def compare_accounts(a, b):
-		if re.split('\W+', a[key])[0].isdigit():
+		if re.split(r'\W+', a[key])[0].isdigit():
 			# if chart of accounts is numbered, then sort by number
 			return cmp(a[key], b[key])
 		elif is_root:

erpnext/accounts/utils.py

@@ -341,31 +341,42 @@ def add_cc(args=None):
 
 def reconcile_against_document(args):
 	"""
-		Cancel JV, Update aginst document, split if required and resubmit jv
+		Cancel PE or JV, Update against document, split if required and resubmit
 	"""
-	for d in args:
+	# To optimize making GL Entry for PE or JV with multiple references
+	reconciled_entries = {}
+	for row in args:
+		if not reconciled_entries.get((row.voucher_type, row.voucher_no)):
+			reconciled_entries[(row.voucher_type, row.voucher_no)] = []
 
-		check_if_advance_entry_modified(d)
+		reconciled_entries[(row.voucher_type, row.voucher_no)].append(row)
-		validate_allocated_amount(d)
+
+	for key, entries in reconciled_entries.items():
+		voucher_type = key[0]
+		voucher_no = key[1]
 
 		# cancel advance entry
-		doc = frappe.get_doc(d.voucher_type, d.voucher_no)
+		doc = frappe.get_doc(voucher_type, voucher_no)
-
 		frappe.flags.ignore_party_validation = True
 		doc.make_gl_entries(cancel=1, adv_adj=1)
 
-		# update ref in advance entry
+		for entry in entries:
-		if d.voucher_type == "Journal Entry":
+			check_if_advance_entry_modified(entry)
-			update_reference_in_journal_entry(d, doc)
+			validate_allocated_amount(entry)
-		else:
-			update_reference_in_payment_entry(d, doc)
 
+			# update ref in advance entry
+			if voucher_type == "Journal Entry":
+				update_reference_in_journal_entry(entry, doc, do_not_save=True)
+			else:
+				update_reference_in_payment_entry(entry, doc, do_not_save=True)
+
+		doc.save(ignore_permissions=True)
 		# re-submit advance entry
-		doc = frappe.get_doc(d.voucher_type, d.voucher_no)
+		doc = frappe.get_doc(entry.voucher_type, entry.voucher_no)
 		doc.make_gl_entries(cancel = 0, adv_adj =1)
 		frappe.flags.ignore_party_validation = False
 
-		if d.voucher_type in ('Payment Entry', 'Journal Entry'):
+		if entry.voucher_type in ('Payment Entry', 'Journal Entry'):
 			doc.update_expense_claim()
 
 def check_if_advance_entry_modified(args):
@@ -374,6 +385,9 @@ def check_if_advance_entry_modified(args):
 		check if amount is same
 		check if jv is submitted
 	"""
+	if not args.get('unreconciled_amount'):
+		args.update({'unreconciled_amount': args.get('unadjusted_amount')})
+
 	ret = None
 	if args.voucher_type == "Journal Entry":
 		ret = frappe.db.sql("""
@@ -395,14 +409,14 @@ def check_if_advance_entry_modified(args):
 					and t1.name = %(voucher_no)s and t2.name = %(voucher_detail_no)s
 					and t1.party_type = %(party_type)s and t1.party = %(party)s and t1.{0} = %(account)s
 					and t2.reference_doctype in ("", "Sales Order", "Purchase Order")
-					and t2.allocated_amount = %(unadjusted_amount)s
+					and t2.allocated_amount = %(unreconciled_amount)s
 			""".format(party_account_field), args)
 		else:
 			ret = frappe.db.sql("""select name from `tabPayment Entry`
 				where
 					name = %(voucher_no)s and docstatus = 1
 					and party_type = %(party_type)s and party = %(party)s and {0} = %(account)s
-					and unallocated_amount = %(unadjusted_amount)s
+					and unallocated_amount = %(unreconciled_amount)s
 			""".format(party_account_field), args)
 
 	if not ret:
@@ -415,58 +429,44 @@ def validate_allocated_amount(args):
 	elif flt(args.get("allocated_amount"), precision) > flt(args.get("unadjusted_amount"), precision):
 		throw(_("Allocated amount cannot be greater than unadjusted amount"))
 
-def update_reference_in_journal_entry(d, jv_obj):
+def update_reference_in_journal_entry(d, journal_entry, do_not_save=False):
 	"""
 		Updates against document, if partial amount splits into rows
 	"""
-	jv_detail = jv_obj.get("accounts", {"name": d["voucher_detail_no"]})[0]
+	jv_detail = journal_entry.get("accounts", {"name": d["voucher_detail_no"]})[0]
-	jv_detail.set(d["dr_or_cr"], d["allocated_amount"])
-	jv_detail.set('debit' if d['dr_or_cr']=='debit_in_account_currency' else 'credit',
-		d["allocated_amount"]*flt(jv_detail.exchange_rate))
-
-	original_reference_type = jv_detail.reference_type
-	original_reference_name = jv_detail.reference_name
-
-	jv_detail.set("reference_type", d["against_voucher_type"])
-	jv_detail.set("reference_name", d["against_voucher"])
-
-	if d['allocated_amount'] < d['unadjusted_amount']:
-		jvd = frappe.db.sql("""
-			select cost_center, balance, against_account, is_advance,
-				account_type, exchange_rate, account_currency
-			from `tabJournal Entry Account` where name = %s
-		""", d['voucher_detail_no'], as_dict=True)
 
+	if flt(d['unadjusted_amount']) - flt(d['allocated_amount']) != 0:
+		# adjust the unreconciled balance
 		amount_in_account_currency = flt(d['unadjusted_amount']) - flt(d['allocated_amount'])
-		amount_in_company_currency = amount_in_account_currency * flt(jvd[0]['exchange_rate'])
+		amount_in_company_currency = amount_in_account_currency * flt(jv_detail.exchange_rate)
+		jv_detail.set(d['dr_or_cr'], amount_in_account_currency)
+		jv_detail.set('debit' if d['dr_or_cr'] == 'debit_in_account_currency' else 'credit', amount_in_company_currency)
+	else:
+		journal_entry.remove(jv_detail)
 
-		# new entry with balance amount
+	# new row with references
-		ch = jv_obj.append("accounts")
+	new_row = journal_entry.append("accounts")
-		ch.account = d['account']
+	new_row.update(jv_detail.as_dict().copy())
-		ch.account_type = jvd[0]['account_type']
-		ch.account_currency = jvd[0]['account_currency']
-		ch.exchange_rate = jvd[0]['exchange_rate']
-		ch.party_type = d["party_type"]
-		ch.party = d["party"]
-		ch.cost_center = cstr(jvd[0]["cost_center"])
-		ch.balance = flt(jvd[0]["balance"])
 
-		ch.set(d['dr_or_cr'], amount_in_account_currency)
+	new_row.set(d["dr_or_cr"], d["allocated_amount"])
-		ch.set('debit' if d['dr_or_cr']=='debit_in_account_currency' else 'credit', amount_in_company_currency)
+	new_row.set('debit' if d['dr_or_cr'] == 'debit_in_account_currency' else 'credit',
+		d["allocated_amount"] * flt(jv_detail.exchange_rate))
 
-		ch.set('credit_in_account_currency' if d['dr_or_cr']== 'debit_in_account_currency'
+	new_row.set('credit_in_account_currency' if d['dr_or_cr'] == 'debit_in_account_currency'
-			else 'debit_in_account_currency', 0)
+		else 'debit_in_account_currency', 0)
-		ch.set('credit' if d['dr_or_cr']== 'debit_in_account_currency' else 'debit', 0)
+	new_row.set('credit' if d['dr_or_cr'] == 'debit_in_account_currency' else 'debit', 0)
 
-		ch.against_account = cstr(jvd[0]["against_account"])
+	new_row.set("reference_type", d["against_voucher_type"])
-		ch.reference_type = original_reference_type
+	new_row.set("reference_name", d["against_voucher"])
-		ch.reference_name = original_reference_name
+
-		ch.is_advance = cstr(jvd[0]["is_advance"])
+	new_row.against_account = cstr(jv_detail.against_account)
-		ch.docstatus = 1
+	new_row.is_advance = cstr(jv_detail.is_advance)
+	new_row.docstatus = 1
 
 	# will work as update after submit
-	jv_obj.flags.ignore_validate_update_after_submit = True
+	journal_entry.flags.ignore_validate_update_after_submit = True
-	jv_obj.save(ignore_permissions=True)
+	if not do_not_save:
+		journal_entry.save(ignore_permissions=True)
 
 def update_reference_in_payment_entry(d, payment_entry, do_not_save=False):
 	reference_details = {

erpnext/agriculture/doctype/disease/disease.py

@@ -6,7 +6,6 @@ from __future__ import unicode_literals
 import frappe
 from frappe import _
 from frappe.model.document import Document
-from frappe import _
 
 class Disease(Document):
 	def validate(self):

erpnext/agriculture/doctype/soil_texture/soil_texture.py

@@ -7,7 +7,6 @@ import frappe
 from frappe import _
 from frappe.model.document import Document
 from frappe.utils import flt, cint
-from frappe import _
 
 class SoilTexture(Document):
 	soil_edit_order = [2, 1, 0]

erpnext/agriculture/doctype/water_analysis/water_analysis.py

@@ -6,7 +6,6 @@ from __future__ import unicode_literals
 import frappe
 from frappe import _
 from frappe.model.document import Document
-from frappe import _
 
 class WaterAnalysis(Document):
 	@frappe.whitelist()

erpnext/assets/doctype/asset/asset.py

@@ -11,7 +11,7 @@ from frappe.model.document import Document
 from erpnext.assets.doctype.asset_category.asset_category import get_asset_category_account
 from erpnext.assets.doctype.asset.depreciation \
 	import get_disposal_account_and_cost_center, get_depreciation_accounts
-from erpnext.accounts.general_ledger import make_gl_entries, make_reverse_gl_entries
+from erpnext.accounts.general_ledger import make_reverse_gl_entries
 from erpnext.accounts.utils import get_account_currency
 from erpnext.controllers.accounts_controller import AccountsController
 
@@ -546,7 +546,7 @@ class Asset(AccountsController):
 		cwip_account = None
 		try:
 			cwip_account = get_asset_account("capital_work_in_progress_account", self.name, self.asset_category, self.company)
-		except:
+		except Exception:
 			# if no cwip account found in category or company and "cwip is enabled" then raise else silently pass
 			if cwip_enabled:
 				raise

erpnext/controllers/accounts_controller.py

@@ -1211,7 +1211,7 @@ class AccountsController(TransactionBase):
 				d.base_payment_amount = flt(base_grand_total * flt(d.invoice_portion / 100), d.precision('base_payment_amount'))
 				d.outstanding = d.payment_amount
 			elif not d.invoice_portion:
-				d.base_payment_amount = flt(base_grand_total * self.get("conversion_rate"), d.precision('base_payment_amount'))
+				d.base_payment_amount = flt(d.payment_amount * self.get("conversion_rate"), d.precision('base_payment_amount'))
 
 
 	def get_order_details(self):
@@ -1592,7 +1592,7 @@ def get_advance_journal_entries(party_type, party, party_account, amount_field,
 
 
 def get_advance_payment_entries(party_type, party, party_account, order_doctype,
-		order_list=None, include_unallocated=True, against_all_orders=False, limit=None):
+		order_list=None, include_unallocated=True, against_all_orders=False, limit=None, condition=None):
 	party_account_field = "paid_from" if party_type == "Customer" else "paid_to"
 	currency_field = "paid_from_account_currency" if party_type == "Customer" else "paid_to_account_currency"
 	payment_type = "Receive" if party_type == "Customer" else "Pay"
@@ -1627,14 +1627,14 @@ def get_advance_payment_entries(party_type, party, party_account, order_doctype,
 
 	if include_unallocated:
 		unallocated_payment_entries = frappe.db.sql("""
-				select "Payment Entry" as reference_type, name as reference_name,
+				select "Payment Entry" as reference_type, name as reference_name, posting_date,
-				remarks, unallocated_amount as amount, {2} as exchange_rate
+				remarks, unallocated_amount as amount, {2} as exchange_rate, {3} as currency
 				from `tabPayment Entry`
 				where
 					{0} = %s and party_type = %s and party = %s and payment_type = %s
-					and docstatus = 1 and unallocated_amount > 0
+					and docstatus = 1 and unallocated_amount > 0 {condition}
 				order by posting_date {1}
-			""".format(party_account_field, limit_cond, exchange_rate_field),
+			""".format(party_account_field, limit_cond, exchange_rate_field, currency_field, condition=condition or ""),
 			(party_account, party_type, party, payment_type), as_dict=1)
 
 	return list(payment_entries_against_order) + list(unallocated_payment_entries)

erpnext/controllers/sales_and_purchase_return.py

@@ -63,7 +63,7 @@ def validate_returned_items(doc):
 
 	if doc.doctype in ("Delivery Note", "Sales Invoice"):
 		for d in frappe.db.sql("""select item_code, qty, serial_no, batch_no from `tabPacked Item`
-			where parent = %s""".format(doc.doctype), doc.return_against, as_dict=1):
+			where parent = %s""", doc.return_against, as_dict=1):
 				valid_items = get_ref_item_dict(valid_items, d)
 
 	already_returned_items = get_already_returned_items(doc)

erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.py

@@ -28,10 +28,10 @@ class AppointmentBookingSettings(Document):
 			to_time = datetime.datetime.strptime(
 				self.min_date+record.to_time, self.format_string)
 			timedelta = to_time-from_time
-			self.validate_from_and_to_time(from_time, to_time)
+			self.validate_from_and_to_time(from_time, to_time, record)
 			self.duration_is_divisible(from_time, to_time)
 
-	def validate_from_and_to_time(self, from_time, to_time):
+	def validate_from_and_to_time(self, from_time, to_time, record):
 		if from_time > to_time:
 			err_msg = _('<b>From Time</b> cannot be later than <b>To Time</b> for {0}').format(record.day_of_week)
 			frappe.throw(_(err_msg))

erpnext/crm/doctype/lead/lead.py

@@ -157,6 +157,7 @@ class Lead(SellingController):
 			"salutation": self.salutation,
 			"gender": self.gender,
 			"designation": self.designation,
+			"company_name": self.company_name,
 		})
 
 		if self.email_id:

erpnext/crm/doctype/social_media_post/social_media_post.py

@@ -67,7 +67,7 @@ class SocialMediaPost(Document):
 				self.db_set("linkedin_post_id", linkedin_post.headers['X-RestLi-Id'])
 			self.db_set("post_status", "Posted")
 
-		except:
+		except Exception:
 			self.db_set("post_status", "Error")
 			title = _("Error while POSTING {0}").format(self.name)
 			frappe.log_error(message=frappe.get_traceback(), title=title)

erpnext/demo/demo.py

@@ -88,7 +88,7 @@ def simulate(domain='Manufacturing', days=100):
 			elif domain=='Education':
 				edu.work()
 
-		except:
+		except Exception:
 			frappe.db.set_global('demo_last_date', current_date)
 			raise
 		finally:

erpnext/education/doctype/course_scheduling_tool/course_scheduling_tool.py

@@ -95,7 +95,7 @@ class CourseSchedulingTool(Document):
 				if self.day == calendar.day_name[getdate(d.schedule_date).weekday()]:
 					frappe.delete_doc("Course Schedule", d.name)
 					rescheduled.append(d.name)
-			except:
+			except Exception:
 				reschedule_errors.append(d.name)
 		return rescheduled, reschedule_errors
 

erpnext/education/utils.py

@@ -219,7 +219,7 @@ def get_quiz(quiz_name, course):
 	try:
 		quiz = frappe.get_doc("Quiz", quiz_name)
 		questions = quiz.get_questions()
-	except:
+	except Exception:
 		frappe.throw(_("Quiz {0} does not exist").format(quiz_name), frappe.DoesNotExistError)
 		return None
 

erpnext/erpnext_integrations/doctype/amazon_mws_settings/xml_utils.py

@@ -88,7 +88,7 @@ class xml2dict(object):
 		ns = http://cs.sfsu.edu/csc867/myscheduler
 		name = patients
 		"""
-		result = re.compile("\{(.*)\}(.*)").search(tag)
+		result = re.compile(r"\{(.*)\}(.*)").search(tag)
 		if result:
 			value.namespace, tag = result.groups()
 

erpnext/erpnext_integrations/doctype/tally_migration/tally_migration.py

@@ -266,7 +266,7 @@ class TallyMigration(Document):
 
 			self.is_master_data_processed = 1
 
-		except:
+		except Exception:
 			self.publish("Process Master Data", _("Process Failed"), -1, 5)
 			self.log()
 
@@ -302,14 +302,14 @@ class TallyMigration(Document):
 				try:
 					party_doc = frappe.get_doc(party)
 					party_doc.insert()
-				except:
+				except Exception:
 					self.log(party_doc)
 			addresses_file = frappe.get_doc("File", {"file_url": addresses_file_url})
 			for address in json.loads(addresses_file.get_content()):
 				try:
 					address_doc = frappe.get_doc(address)
 					address_doc.insert(ignore_mandatory=True)
-				except:
+				except Exception:
 					self.log(address_doc)
 
 		def create_items_uoms(items_file_url, uoms_file_url):
@@ -319,7 +319,7 @@ class TallyMigration(Document):
 					try:
 						uom_doc = frappe.get_doc(uom)
 						uom_doc.insert()
-					except:
+					except Exception:
 						self.log(uom_doc)
 
 			items_file = frappe.get_doc("File", {"file_url": items_file_url})
@@ -327,7 +327,7 @@ class TallyMigration(Document):
 				try:
 					item_doc = frappe.get_doc(item)
 					item_doc.insert()
-				except:
+				except Exception:
 					self.log(item_doc)
 
 		try:
@@ -346,7 +346,7 @@ class TallyMigration(Document):
 			self.is_master_data_imported = 1
 			frappe.db.commit()
 
-		except:
+		except Exception:
 			self.publish("Import Master Data", _("Process Failed"), -1, 5)
 			frappe.db.rollback()
 			self.log()
@@ -370,7 +370,7 @@ class TallyMigration(Document):
 					if processed_voucher:
 						vouchers.append(processed_voucher)
 					frappe.db.commit()
-				except:
+				except Exception:
 					frappe.db.rollback()
 					self.log(voucher)
 			return vouchers
@@ -494,7 +494,7 @@ class TallyMigration(Document):
 
 			self.is_day_book_data_processed = 1
 
-		except:
+		except Exception:
 			self.publish("Process Day Book Data", _("Process Failed"), -1, 5)
 			self.log()
 
@@ -564,7 +564,7 @@ class TallyMigration(Document):
 					is_last = True
 				frappe.enqueue_doc(self.doctype, self.name, "_import_vouchers", queue="long", timeout=3600, start=index+1, total=total, is_last=is_last)
 
-		except:
+		except Exception:
 			self.log()
 
 		finally:
@@ -583,7 +583,7 @@ class TallyMigration(Document):
 				voucher_doc.submit()
 				self.publish("Importing Vouchers", _("{} of {}").format(index, total), index, total)
 				frappe.db.commit()
-			except:
+			except Exception:
 				frappe.db.rollback()
 				self.log(voucher_doc)
 

erpnext/healthcare/doctype/healthcare_service_unit/healthcare_service_unit.py

@@ -30,7 +30,7 @@ class HealthcareServiceUnit(NestedSet):
 		self.validate_one_root()
 
 	def set_service_unit_properties(self):
-		if self.is_group:
+		if cint(self.is_group):
 			self.allow_appointments = False
 			self.overlap_appointments = False
 			self.inpatient_occupancy = False

erpnext/healthcare/doctype/lab_test/lab_test.py

@@ -48,7 +48,7 @@ class LabTest(Document):
 			if item.result_value and item.secondary_uom and item.conversion_factor:
 				try:
 					item.secondary_uom_result = float(item.result_value) * float(item.conversion_factor)
-				except:
+				except Exception:
 					item.secondary_uom_result = ''
 					frappe.msgprint(_('Row #{0}: Result for Secondary UOM not calculated').format(item.idx), title = _('Warning'))
 

erpnext/healthcare/doctype/patient_appointment/patient_appointment.py

@@ -6,7 +6,7 @@ from __future__ import unicode_literals
 import frappe
 from frappe.model.document import Document
 import json
-from frappe.utils import getdate, get_time, flt
+from frappe.utils import getdate, get_time, flt, get_link_to_form
 from frappe.model.mapper import get_mapped_doc
 from frappe import _
 import datetime
@@ -333,17 +333,13 @@ def check_employee_wise_availability(date, practitioner_doc):
 
 
 def get_available_slots(practitioner_doc, date):
-	available_slots = []
+	available_slots = slot_details = []
-	slot_details = []
 	weekday = date.strftime('%A')
 	practitioner = practitioner_doc.name
 
 	for schedule_entry in practitioner_doc.practitioner_schedules:
-		if schedule_entry.schedule:
+		validate_practitioner_schedules(schedule_entry, practitioner)
-			practitioner_schedule = frappe.get_doc('Practitioner Schedule', schedule_entry.schedule)
+		practitioner_schedule = frappe.get_doc('Practitioner Schedule', schedule_entry.schedule)
-		else:
-			frappe.throw(_('{0} does not have a Healthcare Practitioner Schedule. Add it in Healthcare Practitioner').format(
-				frappe.bold(practitioner)), title=_('Practitioner Schedule Not Found'))
 
 		if practitioner_schedule:
 			available_slots = []
@@ -386,6 +382,19 @@ def get_available_slots(practitioner_doc, date):
 	return slot_details
 
 
+def validate_practitioner_schedules(schedule_entry, practitioner):
+	if schedule_entry.schedule:
+		if not schedule_entry.service_unit:
+			frappe.throw(_('Practitioner {0} does not have a Service Unit set against the Practitioner Schedule {1}.').format(
+				get_link_to_form('Healthcare Practitioner', practitioner), frappe.bold(schedule_entry.schedule)),
+				title=_('Service Unit Not Found'))
+
+	else:
+		frappe.throw(_('Practitioner {0} does not have a Practitioner Schedule assigned.').format(
+			get_link_to_form('Healthcare Practitioner', practitioner)),
+			title=_('Practitioner Schedule Not Found'))
+
+
 @frappe.whitelist()
 def update_status(appointment_id, status):
 	frappe.db.set_value('Patient Appointment', appointment_id, 'status', status)

erpnext/healthcare/doctype/patient_encounter/patient_encounter.py

@@ -7,7 +7,6 @@ import frappe
 from frappe import _
 from frappe.model.document import Document
 from frappe.utils import cstr, getdate, add_days
-from frappe import _
 from frappe.model.mapper import get_mapped_doc
 
 

erpnext/hooks.py

@@ -243,6 +243,11 @@ doc_events = {
 		"on_update": ["erpnext.hr.doctype.employee.employee.update_user_permissions",
 			"erpnext.portal.utils.set_default_role"]
 	},
+	"Communication": {
+		"on_update": [
+			"erpnext.support.doctype.issue.issue.set_first_response_time"
+		]
+	},
 	"Sales Taxes and Charges Template": {
 		"on_update": "erpnext.e_commerce.doctype.e_commerce_settings.e_commerce_settings.validate_cart_settings"
 	},

erpnext/hr/doctype/daily_work_summary/daily_work_summary.py

@@ -82,7 +82,7 @@ class DailyWorkSummary(Document):
 							crop=True
 						)
 						d.image = thumbnail_image
-			except:
+			except Exception:
 				d.image = original_image
 
 			if d.sender in did_not_reply:

erpnext/hr/doctype/leave_application/leave_application.py

@@ -662,26 +662,30 @@ def is_lwp(leave_type):
 
 @frappe.whitelist()
 def get_events(start, end, filters=None):
+	from frappe.desk.reportview import get_filters_cond
 	events = []
 
-	employee = frappe.db.get_value("Employee", {"user_id": frappe.session.user}, ["name", "company"],
+	employee = frappe.db.get_value("Employee",
-		as_dict=True)
+		filters={"user_id": frappe.session.user},
+		fieldname=["name", "company"],
+		as_dict=True
+	)
+
 	if employee:
 		employee, company = employee.name, employee.company
 	else:
-		employee=''
+		employee = ''
-		company=frappe.db.get_value("Global Defaults", None, "default_company")
+		company = frappe.db.get_value("Global Defaults", None, "default_company")
 
-	from frappe.desk.reportview import get_filters_cond
 	conditions = get_filters_cond("Leave Application", filters, [])
 	# show department leaves for employee
 	if "Employee" in frappe.get_roles():
 		add_department_leaves(events, start, end, employee, company)
 
 	add_leaves(events, start, end, conditions)
-
 	add_block_dates(events, start, end, employee, company)
 	add_holidays(events, start, end, employee, company)
+
 	return events
 
 def add_department_leaves(events, start, end, employee, company):
@@ -697,26 +701,37 @@ def add_department_leaves(events, start, end, employee, company):
 	filter_conditions = " and employee in (\"%s\")" % '", "'.join(department_employees)
 	add_leaves(events, start, end, filter_conditions=filter_conditions)
 
+
 def add_leaves(events, start, end, filter_conditions=None):
+	from frappe.desk.reportview import build_match_conditions
 	conditions = []
 
-
 	if not cint(frappe.db.get_value("HR Settings", None, "show_leaves_of_all_department_members_in_calendar")):
-		from frappe.desk.reportview import build_match_conditions
 		match_conditions = build_match_conditions("Leave Application")
 
 		if match_conditions:
 			conditions.append(match_conditions)
 
-	query = """select name, from_date, to_date, employee_name, half_day,
+	query = """SELECT
-		status, employee, docstatus
+		docstatus,
-		from `tabLeave Application` where
+		name,
-		from_date <= %(end)s and to_date >= %(start)s <= to_date
+		employee,
-		and docstatus < 2
+		employee_name,
-		and status!='Rejected' """
+		leave_type,
+		from_date,
+		to_date,
+		half_day,
+		status,
+		color
+	FROM `tabLeave Application`
+	WHERE
+		from_date <= %(end)s AND to_date >= %(start)s <= to_date
+		AND docstatus < 2
+		AND status != 'Rejected'
+	"""
 
 	if conditions:
-		query += ' and ' + ' and '.join(conditions)
+		query += ' AND ' + ' AND '.join(conditions)
 
 	if filter_conditions:
 		query += filter_conditions
@@ -729,11 +744,13 @@ def add_leaves(events, start, end, filter_conditions=None):
 			"to_date": d.to_date,
 			"docstatus": d.docstatus,
 			"color": d.color,
-			"title": cstr(d.employee_name) + (' ' + _('(Half Day)') if d.half_day else ''),
+			"all_day": int(not d.half_day),
+			"title": cstr(d.employee_name) + f' ({cstr(d.leave_type)})' + (' ' + _('(Half Day)') if d.half_day else ''),
 		}
 		if e not in events:
 			events.append(e)
 
+
 def add_block_dates(events, start, end, employee, company):
 	# block days
 	from erpnext.hr.doctype.leave_block_list.leave_block_list import get_applicable_block_dates

erpnext/hr/doctype/leave_application/leave_application_calendar.js

@@ -7,7 +7,9 @@ frappe.views.calendar["Leave Application"] = {
 		"end": "to_date",
 		"id": "name",
 		"title": "title",
-		"docstatus": 1
+		"docstatus": 1,
+		"color": "color",
+		"allDay": "all_day"
 	},
 	options: {
 		header: {

erpnext/hr/doctype/leave_control_panel/leave_control_panel.py

@@ -51,7 +51,7 @@ class LeaveControlPanel(Document):
 				la.docstatus = 1
 				la.save()
 				leave_allocated_for.append(d[0])
-			except:
+			except Exception:
 				pass
 		if leave_allocated_for:
 			msgprint(_("Leaves Allocated Successfully for {0}").format(comma_and(leave_allocated_for)))

erpnext/hr/doctype/leave_period/leave_period.py

@@ -5,7 +5,7 @@
 from __future__ import unicode_literals
 import frappe
 from frappe import _
-from frappe.utils import getdate, cstr, add_days, date_diff, getdate, ceil
+from frappe.utils import getdate, cstr, add_days, date_diff, ceil
 from frappe.model.document import Document
 from erpnext.hr.utils import validate_overlap
 from frappe.utils.background_jobs import enqueue

erpnext/hr/page/organizational_chart/organizational_chart.js

@@ -15,6 +15,8 @@ frappe.pages['organizational-chart'].on_page_load = function(wrapper) {
 			} else {
 				organizational_chart = new erpnext.HierarchyChart('Employee', wrapper, method);
 			}
+
+			frappe.breadcrumbs.add('HR');
 			organizational_chart.show();
 		});
 	});

erpnext/hr/report/employee_leave_balance_summary/employee_leave_balance_summary.py

@@ -65,7 +65,7 @@ def get_data(filters, leave_types):
 			for leave_type in leave_types:
 				remaining = 0
 				if leave_type in available_leave["leave_allocation"]:
-				# opening balance
+					# opening balance
 					remaining = available_leave["leave_allocation"][leave_type]['remaining_leaves']
 
 				row += [remaining]

erpnext/hr/report/vehicle_expenses/vehicle_expenses.py

@@ -96,8 +96,6 @@ def get_columns():
 		}
 	]
 
-	return columns
-
 
 def get_vehicle_log_data(filters):
 	start_date, end_date = get_period_dates(filters)

erpnext/loan_management/doctype/loan_interest_accrual/loan_interest_accrual.py

@@ -7,7 +7,7 @@ import frappe, erpnext
 from frappe import _
 from frappe.model.document import Document
 from frappe.utils import (nowdate, getdate, now_datetime, get_datetime, flt, date_diff, get_last_day, cint,
-	get_first_day, get_datetime, add_days)
+	get_first_day, add_days)
 from erpnext.controllers.accounts_controller import AccountsController
 from erpnext.accounts.general_ledger import make_gl_entries
 

erpnext/loan_management/doctype/loan_repayment/loan_repayment.py

@@ -6,10 +6,9 @@ from __future__ import unicode_literals
 import frappe, erpnext
 import json
 from frappe import _
-from frappe.utils import flt, getdate, cint
 from six import iteritems
 from frappe.model.document import Document
-from frappe.utils import date_diff, add_days, getdate, add_months, get_first_day, get_datetime
+from frappe.utils import flt, cint, date_diff, add_days, getdate, add_months, get_first_day, get_datetime
 from erpnext.controllers.accounts_controller import AccountsController
 from erpnext.accounts.general_ledger import make_gl_entries
 from erpnext.loan_management.doctype.loan_security_shortfall.loan_security_shortfall import update_shortfall_status
@@ -107,12 +106,13 @@ class LoanRepayment(AccountsController):
 				lia = frappe.db.get_value('Loan Interest Accrual', {'process_loan_interest_accrual':
 					process}, ['name', 'interest_amount', 'payable_principal_amount'], as_dict=1)
 
-				self.append('repayment_details', {
+				if lia:
-					'loan_interest_accrual': lia.name,
+					self.append('repayment_details', {
-					'paid_interest_amount': flt(self.total_interest_paid - self.interest_payable, precision),
+						'loan_interest_accrual': lia.name,
-					'paid_principal_amount': 0.0,
+						'paid_interest_amount': flt(self.total_interest_paid - self.interest_payable, precision),
-					'accrual_type': 'Repayment'
+						'paid_principal_amount': 0.0,
-				})
+						'accrual_type': 'Repayment'
+					})
 
 	def update_paid_amount(self):
 		loan = frappe.get_doc("Loan", self.against_loan)

erpnext/loan_management/doctype/loan_security_shortfall/loan_security_shortfall.py

@@ -71,7 +71,7 @@ def check_for_ltv_shortfall(process_loan_security_shortfall):
 				- flt(loan.total_principal_paid)
 
 		pledged_securities = get_pledged_security_qty(loan.name)
-		ltv_ratio = ''
+		ltv_ratio = 0.0
 		security_value = 0.0
 
 		for security, qty in pledged_securities.items():

erpnext/loan_management/doctype/sanctioned_loan_amount/sanctioned_loan_amount.py

@@ -4,6 +4,7 @@
 
 from __future__ import unicode_literals
 import frappe
+from frappe import _
 from frappe.model.document import Document
 
 class SanctionedLoanAmount(Document):

erpnext/manufacturing/doctype/material_request_plan_item/material_request_plan_item.json

@@ -6,17 +6,17 @@
  "engine": "InnoDB",
  "field_order": [
   "item_code",
-  "item_name",
-  "material_request_type",
   "from_warehouse",
   "warehouse",
-  "column_break_4",
+  "item_name",
+  "material_request_type",
+  "actual_qty",
+  "ordered_qty",
   "required_bom_qty",
+  "column_break_4",
   "quantity",
   "uom",
   "projected_qty",
-  "actual_qty",
-  "ordered_qty",
   "reserved_qty_for_production",
   "safety_stock",
   "item_details",
@@ -28,6 +28,7 @@
  ],
  "fields": [
   {
+   "columns": 2,
    "fieldname": "item_code",
    "fieldtype": "Link",
    "in_list_view": 1,
@@ -41,6 +42,7 @@
    "label": "Item Name"
   },
   {
+   "columns": 2,
    "fieldname": "warehouse",
    "fieldtype": "Link",
    "in_list_view": 1,
@@ -50,10 +52,11 @@
    "reqd": 1
   },
   {
+   "columns": 1,
    "fieldname": "material_request_type",
    "fieldtype": "Select",
    "in_list_view": 1,
-   "label": "Material Request Type",
+   "label": "Type",
    "options": "\nPurchase\nMaterial Transfer\nMaterial Issue\nManufacture\nCustomer Provided"
   },
   {
@@ -61,10 +64,11 @@
    "fieldtype": "Column Break"
   },
   {
+   "columns": 1,
    "fieldname": "quantity",
    "fieldtype": "Float",
    "in_list_view": 1,
-   "label": "Required Quantity",
+   "label": "Plan to Request Qty",
    "no_copy": 1,
    "reqd": 1
   },
@@ -75,11 +79,12 @@
    "read_only": 1
   },
   {
+   "columns": 2,
    "default": "0",
    "fieldname": "actual_qty",
    "fieldtype": "Float",
    "in_list_view": 1,
-   "label": "Actual Qty",
+   "label": "Available Qty",
    "no_copy": 1,
    "read_only": 1
   },
@@ -157,16 +162,18 @@
    "read_only": 1
   },
   {
+   "columns": 2,
    "fieldname": "required_bom_qty",
    "fieldtype": "Float",
-   "label": "Required Qty as per BOM",
+   "in_list_view": 1,
+   "label": "Qty As Per BOM",
    "no_copy": 1,
    "read_only": 1
   }
  ],
  "istable": 1,
  "links": [],
- "modified": "2021-03-26 12:41:13.013149",
+ "modified": "2021-08-23 18:17:58.400462",
  "modified_by": "Administrator",
  "module": "Manufacturing",
  "name": "Material Request Plan Item",

erpnext/manufacturing/doctype/production_plan/production_plan.js

@@ -254,7 +254,7 @@ frappe.ui.form.on('Production Plan', {
 
 	get_items_for_mr: function(frm) {
 		if (!frm.doc.for_warehouse) {
-			frappe.throw(__("Select warehouse for material requests"));
+			frappe.throw(__("To make material requests, 'Make Material Request for Warehouse' field is mandatory"));
 		}
 
 		if (frm.doc.ignore_existing_ordered_qty) {
@@ -265,9 +265,18 @@ frappe.ui.form.on('Production Plan', {
 				title: title,
 				fields: [
 					{
-						"fieldtype": "Table MultiSelect", "label": __("Source Warehouses (Optional)"),
+						'label': __('Target Warehouse'),
-						"fieldname": "warehouses", "options": "Production Plan Material Request Warehouse",
+						'fieldtype': 'Link',
-						"description": __("System will pickup the materials from the selected warehouses. If not specified, system will create material request for purchase."),
+						'fieldname': 'target_warehouse',
+						'read_only': true,
+						'default': frm.doc.for_warehouse
+					},
+					{
+						'label': __('Source Warehouses (Optional)'),
+						'fieldtype': 'Table MultiSelect',
+						'fieldname': 'warehouses',
+						'options': 'Production Plan Material Request Warehouse',
+						'description': __('If source warehouse selected then system will create the material request with type Material Transfer from Source to Target warehouse. If not selected then will create the material request with type Purchase for the target warehouse.'),
 						get_query: function () {
 							return {
 								filters: {
@@ -342,7 +351,11 @@ frappe.ui.form.on('Production Plan', {
 
 		frappe.prompt(fields, (row) => {
 			let get_template_url = 'erpnext.manufacturing.doctype.production_plan.production_plan.download_raw_materials';
-			open_url_post(frappe.request.url, { cmd: get_template_url, doc: frm.doc, warehouses: row.warehouses });
+			open_url_post(frappe.request.url, {
+				cmd: get_template_url,
+				doc: frm.doc,
+				warehouses: row.warehouses
+			});
 		}, __('Select Warehouses to get Stock for Materials Planning'), __('Get Stock'));
 	},
 

erpnext/manufacturing/doctype/production_plan/production_plan.json

@@ -300,7 +300,7 @@
   {
    "fieldname": "for_warehouse",
    "fieldtype": "Link",
-   "label": "Material Request Warehouse",
+   "label": "Make Material Request for Warehouse",
    "options": "Warehouse"
   },
   {
@@ -364,7 +364,7 @@
  "index_web_pages_for_search": 1,
  "is_submittable": 1,
  "links": [],
- "modified": "2021-06-28 20:00:33.905114",
+ "modified": "2021-08-23 17:26:03.799876",
  "modified_by": "Administrator",
  "module": "Manufacturing",
  "name": "Production Plan",

erpnext/manufacturing/doctype/production_plan/production_plan.py

@@ -331,7 +331,7 @@ class ProductionPlan(Document):
 	def get_production_items(self):
 		item_dict = {}
 		for d in self.po_items:
-			item_details= {
+			item_details = {
 				"production_item"		: d.item_code,
 				"use_multi_level_bom"   : d.include_exploded_items,
 				"sales_order"			: d.sales_order,
@@ -346,8 +346,7 @@ class ProductionPlan(Document):
 				"production_plan"       : self.name,
 				"production_plan_item"  : d.name,
 				"product_bundle_item"	: d.product_bundle_item,
-				"planned_start_date"    : d.planned_start_date,
+				"planned_start_date"    : d.planned_start_date
-				"make_work_order_for_sub_assembly_items": d.get("make_work_order_for_sub_assembly_items", 0)
 			}
 
 			item_details.update({
@@ -458,6 +457,7 @@ class ProductionPlan(Document):
 		warehouse = get_default_warehouse()
 		wo = frappe.new_doc("Work Order")
 		wo.update(item)
+		wo.planned_start_date = item.get('planned_start_date') or item.get('schedule_date')
 
 		if item.get("warehouse"):
 			wo.fg_warehouse = item.get("warehouse")
@@ -569,7 +569,10 @@ def download_raw_materials(doc, warehouses=None):
 		'Reserved Qty for Production', 'Safety Stock', 'Required Qty']]
 
 	doc.warehouse = None
-	for d in get_items_for_material_requests(doc, warehouses=warehouses, get_parent_warehouse_data=True):
+	frappe.flags.show_qty_in_stock_uom = 1
+	items = get_items_for_material_requests(doc, warehouses=warehouses, get_parent_warehouse_data=True)
+
+	for d in items:
 		item_list.append([d.get('item_code'), d.get('description'), d.get('stock_uom'), d.get('warehouse'),
 			d.get('required_bom_qty'), d.get('projected_qty'), d.get('actual_qty'), d.get('ordered_qty'),
 			d.get('planned_qty'), d.get('reserved_qty_for_production'), d.get('safety_stock'), d.get('quantity')])
@@ -605,9 +608,16 @@ def get_exploded_items(item_details, company, bom_no, include_non_stock_items, p
 			and bom.name=%s and item.is_stock_item in (1, {0})
 		group by bei.item_code, bei.stock_uom""".format(0 if include_non_stock_items else 1),
 		(planned_qty, company, bom_no), as_dict=1):
-			item_details.setdefault(d.get('item_code'), d)
+		if not d.conversion_factor and d.purchase_uom:
+			d.conversion_factor = get_uom_conversion_factor(d.item_code, d.purchase_uom)
+		item_details.setdefault(d.get('item_code'), d)
+
 	return item_details
 
+def get_uom_conversion_factor(item_code, uom):
+	return frappe.db.get_value('UOM Conversion Detail',
+		{'parent': item_code, 'uom': uom}, 'conversion_factor')
+
 def get_subitems(doc, data, item_details, bom_no, company, include_non_stock_items,
 	include_subcontracted_items, parent_qty, planned_qty=1):
 	items = frappe.db.sql("""
@@ -642,6 +652,9 @@ def get_subitems(doc, data, item_details, bom_no, company, include_non_stock_ite
 			if d.item_code in item_details:
 				item_details[d.item_code].qty = item_details[d.item_code].qty + d.qty
 			else:
+				if not d.conversion_factor and d.purchase_uom:
+					d.conversion_factor = get_uom_conversion_factor(d.item_code, d.purchase_uom)
+
 				item_details[d.item_code] = d
 
 		if data.get('include_exploded_items') and d.default_bom:
@@ -669,10 +682,11 @@ def get_material_request_items(row, sales_order, company,
 		row['purchase_uom'] = row['stock_uom']
 
 	if row['purchase_uom'] != row['stock_uom']:
-		if not row['conversion_factor']:
+		if not (row['conversion_factor'] or frappe.flags.show_qty_in_stock_uom):
 			frappe.throw(_("UOM Conversion factor ({0} -> {1}) not found for item: {2}")
 				.format(row['purchase_uom'], row['stock_uom'], row.item_code))
-		required_qty = required_qty / row['conversion_factor']
+
+			required_qty = required_qty / row['conversion_factor']
 
 	if frappe.db.get_value("UOM", row['purchase_uom'], "must_be_whole_number"):
 		required_qty = ceil(required_qty)
@@ -841,10 +855,8 @@ def get_items_for_material_requests(doc, warehouses=None, get_parent_warehouse_d
 		elif data.get('item_code'):
 			item_master = frappe.get_doc('Item', data['item_code']).as_dict()
 			purchase_uom = item_master.purchase_uom or item_master.stock_uom
-			conversion_factor = 0
+			conversion_factor = (get_uom_conversion_factor(item_master.name, purchase_uom)
-			for d in item_master.get("uoms"):
+				if item_master.purchase_uom else 1.0)
-				if d.uom == purchase_uom:
-					conversion_factor = d.conversion_factor
 
 			item_details[item_master.name] = frappe._dict(
 				{

erpnext/manufacturing/report/bom_stock_calculated/bom_stock_calculated.py

@@ -7,7 +7,7 @@ from frappe import _
 from frappe.utils.data import comma_and
 
 def execute(filters=None):
-#	if not filters: filters = {}
+	# if not filters: filters = {}
 	columns = get_columns()
 	summ_data = []
 

erpnext/non_profit/doctype/membership/membership.py

@@ -207,7 +207,7 @@ def get_member_based_on_subscription(subscription_id, email=None, customer_id=No
 
 	try:
 		return frappe.get_doc("Member", members[0]["name"])
-	except:
+	except Exception:
 		return None
 
 
@@ -393,7 +393,7 @@ def notify_failure(log):
 		""".format(get_link_to_form("Error Log", log.name))
 
 		sendmail_to_system_managers("[Important] [ERPNext] Razorpay membership webhook failed , please check.", content)
-	except:
+	except Exception:
 		pass
 
 
@@ -402,7 +402,7 @@ def get_plan_from_razorpay_id(plan_id):
 
 	try:
 		return plan[0]["name"]
-	except:
+	except Exception:
 		return None
 
 

erpnext/patches/v11_0/move_item_defaults_to_child_table_for_multicompany.py

@@ -30,7 +30,7 @@ def execute():
 						buying_cost_center, selling_cost_center, expense_account, income_account, default_supplier
 					FROM `tabItem`;
 			''', companies[0].name)
-		except:
+		except Exception:
 			pass
 	else:
 		item_details = frappe.db.sql(""" SELECT name, default_warehouse,

erpnext/patches/v12_0/update_is_cancelled_field.py

@@ -11,5 +11,5 @@ def execute():
 
 		frappe.reload_doc("stock", "doctype", "stock_ledger_entry")
 		frappe.reload_doc("stock", "doctype", "serial_no")
-	except:
+	except Exception:
 		pass

erpnext/patches/v13_0/move_tax_slabs_from_payroll_period_to_income_tax_slab.py

@@ -86,7 +86,7 @@ def execute():
 			try:
 				employee_other_income.submit()
 				migrated.append([proof.employee, proof.payroll_period])
-			except:
+			except Exception:
 				pass
 
 	if not frappe.db.table_exists("Employee Tax Exemption Declaration"):
@@ -108,5 +108,5 @@ def execute():
 
 			try:
 				employee_other_income.submit()
-			except:
+			except Exception:
 				pass

erpnext/patches/v4_2/update_requested_and_ordered_qty.py

@@ -20,5 +20,5 @@ def execute():
 				})
 				if count % 200 == 0:
 					frappe.db.commit()
-			except:
+			except Exception:
 				frappe.db.rollback()

erpnext/payroll/doctype/gratuity/gratuity.py

@@ -242,7 +242,11 @@ def get_salary_structure(employee):
 		order_by = "from_date desc")[0].salary_structure
 
 def get_last_salary_slip(employee):
-	return frappe.get_list("Salary Slip", filters = {
+	salary_slips = frappe.get_list("Salary Slip", filters = {
 			"employee": employee, 'docstatus': 1
 		},
-		order_by = "start_date desc")[0].name
+		order_by = "start_date desc"
+	)
+	if not salary_slips:
+		return
+	return salary_slips[0].name

erpnext/payroll/doctype/gratuity/test_gratuity.py

@@ -24,6 +24,11 @@ class TestGratuity(unittest.TestCase):
 		frappe.db.sql("DELETE FROM `tabGratuity`")
 		frappe.db.sql("DELETE FROM `tabAdditional Salary` WHERE ref_doctype = 'Gratuity'")
 
+	def test_get_last_salary_slip_should_return_none_for_new_employee(self):
+		new_employee = make_employee("new_employee@salary.com", company='_Test Company')
+		salary_slip = get_last_salary_slip(new_employee)
+		assert salary_slip is None
+
 	def test_check_gratuity_amount_based_on_current_slab_and_additional_salary_creation(self):
 		employee, sal_slip = create_employee_and_get_last_salary_slip()
 

erpnext/payroll/doctype/salary_slip/salary_slip.js

@@ -135,15 +135,15 @@ frappe.ui.form.on("Salary Slip", {
 
 	change_form_labels: function(frm, company_currency) {
 		frm.set_currency_labels(["base_hour_rate", "base_gross_pay", "base_total_deduction",
-			"base_net_pay", "base_rounded_total", "base_total_in_words", "base_year_to_date", "base_month_to_date"],
+			"base_net_pay", "base_rounded_total", "base_total_in_words", "base_year_to_date", "base_month_to_date", "gross_base_year_to_date"],
 		company_currency);
 
-		frm.set_currency_labels(["hour_rate", "gross_pay", "total_deduction", "net_pay", "rounded_total", "total_in_words", "year_to_date", "month_to_date"],
+		frm.set_currency_labels(["hour_rate", "gross_pay", "total_deduction", "net_pay", "rounded_total", "total_in_words", "year_to_date", "month_to_date", "gross_year_to_date"],
 			frm.doc.currency);
 
 		// toggle fields
 		frm.toggle_display(["exchange_rate", "base_hour_rate", "base_gross_pay", "base_total_deduction",
-			"base_net_pay", "base_rounded_total", "base_total_in_words", "base_year_to_date", "base_month_to_date"],
+			"base_net_pay", "base_rounded_total", "base_total_in_words", "base_year_to_date", "base_month_to_date", "base_gross_year_to_date"],
 		frm.doc.currency != company_currency);
 	},
 

erpnext/payroll/doctype/salary_slip/salary_slip.json

@@ -56,6 +56,8 @@
   "totals",
   "gross_pay",
   "base_gross_pay",
+  "gross_year_to_date",
+  "base_gross_year_to_date",
   "column_break_25",
   "total_deduction",
   "base_total_deduction",
@@ -625,13 +627,27 @@
    "label": "Leave Details",
    "options": "Salary Slip Leave",
    "read_only": 1
+  },
+  {
+   "fieldname": "gross_year_to_date",
+   "fieldtype": "Currency",
+   "label": "Gross Year To Date",
+   "options": "currency",
+   "read_only": 1
+  },
+  {
+   "fieldname": "base_gross_year_to_date",
+   "fieldtype": "Currency",
+   "label": "Gross Year To Date(Company Currency)",
+   "options": "Company:company:default_currency",
+   "read_only": 1
   }
  ],
  "icon": "fa fa-file-text",
  "idx": 9,
  "is_submittable": 1,
  "links": [],
- "modified": "2021-03-31 22:44:09.772331",
+ "modified": "2021-09-01 10:22:52.374549",
  "modified_by": "Administrator",
  "module": "Payroll",
  "name": "Salary Slip",

erpnext/payroll/doctype/salary_slip/salary_slip.py

@@ -1218,7 +1218,7 @@ class SalarySlip(TransactionBase):
 		period_start_date, period_end_date = self.get_year_to_date_period()
 
 		salary_slip_sum = frappe.get_list('Salary Slip',
-			fields = ['sum(net_pay) as sum'],
+			fields = ['sum(net_pay) as net_sum', 'sum(gross_pay) as gross_sum'],
 			filters = {'employee_name' : self.employee_name,
 				'start_date' : ['>=', period_start_date],
 				'end_date' : ['<', period_end_date],
@@ -1226,10 +1226,13 @@ class SalarySlip(TransactionBase):
 				'docstatus': 1
 			})
 
-		year_to_date = flt(salary_slip_sum[0].sum) if salary_slip_sum else 0.0
+		year_to_date = flt(salary_slip_sum[0].net_sum) if salary_slip_sum else 0.0
+		gross_year_to_date = flt(salary_slip_sum[0].gross_sum) if salary_slip_sum else 0.0
 
 		year_to_date += self.net_pay
+		gross_year_to_date += self.gross_pay
 		self.year_to_date = year_to_date
+		self.gross_year_to_date = gross_year_to_date
 
 	def compute_month_to_date(self):
 		month_to_date = 0

erpnext/public/js/controllers/transaction.js

@@ -846,21 +846,25 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({
 
 		if (frappe.meta.get_docfield(this.frm.doctype, "shipping_address") &&
 			in_list(['Purchase Order', 'Purchase Receipt', 'Purchase Invoice'], this.frm.doctype)) {
-			erpnext.utils.get_shipping_address(this.frm, function(){
+			erpnext.utils.get_shipping_address(this.frm, function() {
 				set_party_account(set_pricing);
 			});
 
 			// Get default company billing address in Purchase Invoice, Order and Receipt
-			frappe.call({
+			if (this.frm.doc.company && frappe.meta.get_docfield(this.frm.doctype, "billing_address")) {
-				'method': 'frappe.contacts.doctype.address.address.get_default_address',
+				frappe.call({
-				'args': {
+					method: "erpnext.setup.doctype.company.company.get_default_company_address",
-					'doctype': 'Company',
+					args: {name: this.frm.doc.company, existing_address: this.frm.doc.billing_address || ""},
-					'name': this.frm.doc.company
+					debounce: 2000,
-				},
+					callback: function(r) {
-				'callback': function(r) {
+						if (r.message) {
-					me.frm.set_value('billing_address', r.message);
+							me.frm.set_value("billing_address", r.message);
-				}
+						} else {
-			});
+							me.frm.set_value("company_address", "");
+						}
+					}
+				});
+			}
 
 		} else {
 			set_party_account(set_pricing);

erpnext/public/js/hierarchy_chart/hierarchy_chart_desktop.js

@@ -67,8 +67,6 @@ erpnext.HierarchyChart = class {
 	}
 
 	show() {
-		frappe.breadcrumbs.add('HR');
-
 		this.setup_actions();
 		if ($(`[data-fieldname="company"]`).length) return;
 		let me = this;
@@ -83,8 +81,9 @@ erpnext.HierarchyChart = class {
 			reqd: 1,
 			change: () => {
 				me.company = undefined;
+				$('#hierarchy-chart-wrapper').remove();
 
-				if (company.get_value() && me.company != company.get_value()) {
+				if (company.get_value()) {
 					me.company = company.get_value();
 
 					// svg for connectors
@@ -92,6 +91,8 @@ erpnext.HierarchyChart = class {
 					me.setup_hierarchy();
 					me.render_root_nodes();
 					me.all_nodes_expanded = false;
+				} else {
+					frappe.throw(__('Please select a company first.'));
 				}
 			}
 		});
@@ -172,11 +173,11 @@ erpnext.HierarchyChart = class {
 			</ul>`);
 
 		this.page.main
-			.find('#hierarchy-chart-wrapper')
+			.find('#hierarchy-chart')
+			.empty()
 			.append(this.$hierarchy);
 
 		this.nodes = {};
-		this.all_nodes_expanded = false;
 	}
 
 	make_svg_markers() {
@@ -203,6 +204,8 @@ erpnext.HierarchyChart = class {
 					<g id="connectors" fill="none">
 					</g>
 				</svg>
+				<div id="hierarchy-chart">
+				</div>
 			</div>`);
 	}
 
@@ -219,7 +222,10 @@ erpnext.HierarchyChart = class {
 				let expand_node = undefined;
 				let node = undefined;
 
-				$.each(r.message, (i, data) => {
+				$.each(r.message, (_i, data) => {
+					if ($(`#${data.id}`).length)
+						return;
+
 					node = new me.Node({
 						id: data.id,
 						parent: $('<li class="child-node"></li>').appendTo(me.$hierarchy.find('.node-children')),
@@ -290,7 +296,7 @@ erpnext.HierarchyChart = class {
 				() => frappe.dom.freeze(),
 				() => this.setup_hierarchy(),
 				() => this.render_root_nodes(true),
-				() => this.get_all_nodes(node.id, node.name),
+				() => this.get_all_nodes(),
 				(data_list) => this.render_children_of_all_nodes(data_list),
 				() => frappe.dom.unfreeze()
 			]);
@@ -341,15 +347,13 @@ erpnext.HierarchyChart = class {
 		node.expanded = true;
 	}
 
-	get_all_nodes(node_id, node_name) {
+	get_all_nodes() {
 		return new Promise(resolve => {
 			frappe.call({
 				method: 'erpnext.utilities.hierarchy_chart.get_all_nodes',
 				args: {
 					method: this.method,
-					company: this.company,
+					company: this.company
-					parent: node_id,
-					parent_name: node_name
 				},
 				callback: (r) => {
 					resolve(r.message);

erpnext/public/js/hierarchy_chart/hierarchy_chart_mobile.js

@@ -59,8 +59,6 @@ erpnext.HierarchyChartMobile = class {
 	}
 
 	show() {
-		frappe.breadcrumbs.add('HR');
-
 		let me = this;
 		if ($(`[data-fieldname="company"]`).length) return;
 

erpnext/public/js/utils/party.js

@@ -289,8 +289,8 @@ erpnext.utils.get_shipping_address = function(frm, callback) {
 				company: frm.doc.company,
 				address: frm.doc.shipping_address
 			},
-			callback: function(r){
+			callback: function(r) {
-				if (r.message){
+				if (r.message) {
 					frm.set_value("shipping_address", r.message[0]) //Address title or name
 					frm.set_value("shipping_address_display", r.message[1]) //Address to be displayed on the page
 				}

erpnext/regional/report/eway_bill/eway_bill.py

@@ -41,7 +41,7 @@ def get_data(filters):
 	}
 
 	# Regular expression set to remove all the special characters
-	special_characters = "[$%^*()+\\[\]{};':\"\\|<>.?]"
+	special_characters = r"[$%^*()+\\[\]{};':\"\\|<>.?]"
 
 	for row in data:
 		set_defaults(row)

erpnext/regional/report/fichier_des_ecritures_comptables_[fec]/fichier_des_ecritures_comptables_[fec].py

@@ -116,7 +116,7 @@ def get_result_as_list(data, filters):
 		if d.get("voucher_no").startswith("{0}-".format(JournalCode)) or d.get("voucher_no").startswith("{0}/".format(JournalCode)):
 			EcritureNum = re.split("-|/", d.get("voucher_no"))[1]
 		else:
-			EcritureNum = re.search("{0}(\d+)".format(JournalCode), d.get("voucher_no"), re.IGNORECASE).group(1)
+			EcritureNum = re.search(r"{0}(\d+)".format(JournalCode), d.get("voucher_no"), re.IGNORECASE).group(1)
 
 		EcritureDate = format_datetime(d.get("GlPostDate"), "yyyyMMdd")
 

erpnext/selling/page/point_of_sale/point_of_sale.py

@@ -2,7 +2,8 @@
 # License: GNU General Public License v3. See license.txt
 
 from __future__ import unicode_literals
-import frappe, json
+import json
+import frappe
 from frappe.utils.nestedset import get_root_of
 from frappe.utils import cint
 from erpnext.accounts.doctype.pos_profile.pos_profile import get_item_groups
@@ -209,7 +210,6 @@ def check_opening_entry(user):
 
 @frappe.whitelist()
 def create_opening_voucher(pos_profile, company, balance_details):
-	import json
 	balance_details = json.loads(balance_details)
 
 	new_pos_opening = frappe.get_doc({

erpnext/selling/page/point_of_sale/pos_payment.js

@@ -253,41 +253,6 @@ erpnext.PointOfSale.Payment = class {
 		}
 	}
 
-	setup_listener_for_payments() {
-		frappe.realtime.on("process_phone_payment", (data) => {
-			const doc = this.events.get_frm().doc;
-			const { response, amount, success, failure_message } = data;
-			let message, title;
-
-			if (success) {
-				title = __("Payment Received");
-				if (amount >= doc.grand_total) {
-					frappe.dom.unfreeze();
-					message = __("Payment of {0} received successfully.", [format_currency(amount, doc.currency, 0)]);
-					this.events.submit_invoice();
-					cur_frm.reload_doc();
-
-				} else {
-					message = __("Payment of {0} received successfully. Waiting for other requests to complete...", [format_currency(amount, doc.currency, 0)]);
-				}
-			} else if (failure_message) {
-				message = failure_message;
-				title = __("Payment Failed");
-			}
-
-			frappe.msgprint({ "message": message, "title": title });
-		});
-	}
-
-	auto_set_remaining_amount() {
-		const doc = this.events.get_frm().doc;
-		const remaining_amount = doc.grand_total - doc.paid_amount;
-		const current_value = this.selected_mode ? this.selected_mode.get_value() : undefined;
-		if (!current_value && remaining_amount > 0 && this.selected_mode) {
-			this.selected_mode.set_value(remaining_amount);
-		}
-	}
-
 	attach_shortcuts() {
 		const ctrl_label = frappe.utils.is_mac() ? '⌘' : 'Ctrl';
 		this.$component.find('.submit-order-btn').attr("title", `${ctrl_label}+Enter`);

erpnext/selling/report/sales_partner_target_variance_based_on_item_group/item_group_wise_sales_target_variance.py

@@ -44,18 +44,6 @@ def get_data(filters, period_list, partner_doctype):
 		if d.item_group not in item_groups:
 			item_groups.append(d.item_group)
 
-	if item_groups:
-		child_items = []
-		for item_group in item_groups:
-			if frappe.db.get_value("Item Group", {"name":item_group}, "is_group"):
-				for child_item_group in frappe.get_all("Item Group", {"parent_item_group":item_group}):
-					if child_item_group['name'] not in child_items:
-						child_items.append(child_item_group['name'])
-
-		for item in child_items:
-			if item not in item_groups:
-				item_groups.append(item)
-
 	date_field = ("transaction_date"
 		if filters.get('doctype') == "Sales Order" else "posting_date")
 

erpnext/setup/doctype/email_digest/email_digest.py

@@ -60,9 +60,6 @@ class EmailDigest(Document):
 						reference_name = self.name,
 						unsubscribe_message = _("Unsubscribe from this Email Digest"))
 
-		frappe.set_user(original_user)
-		frappe.set_user_lang(original_user)
-
 	def get_msg_html(self):
 		"""Build email digest content"""
 		frappe.flags.ignore_account_permission = True

erpnext/setup/doctype/naming_series/naming_series.py

@@ -79,7 +79,8 @@ class NamingSeries(Document):
 		options = self.scrub_options_list(ol)
 
 		# validate names
-		for i in options: self.validate_series_name(i)
+		for i in options:
+			self.validate_series_name(i)
 
 		if options and self.user_must_always_select:
 			options = [''] + options
@@ -138,7 +139,7 @@ class NamingSeries(Document):
 
 	def validate_series_name(self, n):
 		import re
-		if not re.match("^[\w\- /.#{}]*$", n, re.UNICODE):
+		if not re.match(r"^[\w\- \/.#{}]+$", n, re.UNICODE):
 			throw(_('Special Characters except "-", "#", ".", "/", "{" and "}" not allowed in naming series'))
 
 	@frappe.whitelist()

erpnext/setup/setup_wizard/setup_wizard.py

@@ -106,7 +106,7 @@ def fin(args):
 def make_sample_data(domains):
 	try:
 		sample_data.make_sample_data(domains)
-	except:
+	except Exception:
 		# clear message
 		if frappe.message_log:
 			frappe.message_log.pop()

erpnext/setup/utils.py

@@ -109,7 +109,7 @@ def get_exchange_rate(from_currency, to_currency, transaction_date=None, args=No
 			value = response.json()["result"]
 			cache.setex(name=key, time=21600, value=flt(value))
 		return flt(value)
-	except:
+	except Exception:
 		frappe.log_error(title="Get Exchange Rate")
 		frappe.msgprint(_("Unable to find exchange rate for {0} to {1} for key date {2}. Please create a Currency Exchange record manually").format(from_currency, to_currency, transaction_date))
 		return 0.0

erpnext/stock/doctype/delivery_note/delivery_note.py

@@ -331,7 +331,7 @@ class DeliveryNote(SellingController):
 			credit_note_link = frappe.utils.get_link_to_form('Sales Invoice', return_invoice.name)
 
 			frappe.msgprint(_("Credit Note {0} has been created automatically").format(credit_note_link))
-		except:
+		except Exception:
 			frappe.throw(_("Could not create Credit Note automatically, please uncheck 'Issue Credit Note' and submit again"))
 
 def update_billed_amount_based_on_so(so_detail, update_modified=True):

erpnext/stock/doctype/serial_no/serial_no.py

@@ -573,7 +573,7 @@ def auto_fetch_serial_number(qty, item_code, warehouse, posting_date=None, batch
 	if batch_nos:
 		try:
 			filters["batch_no"] = json.loads(batch_nos) if (type(json.loads(batch_nos)) == list) else [json.loads(batch_nos)]
-		except:
+		except Exception:
 			filters["batch_no"] = [batch_nos]
 
 	if posting_date:

erpnext/stock/doctype/stock_entry/stock_entry.py

@@ -1200,10 +1200,10 @@ class StockEntry(StockController):
 
 			wo_item_qty = item.transferred_qty or item.required_qty
 
-			req_qty_each = (
+			wo_qty_consumed = flt(wo_item_qty) - flt(item.consumed_qty)
-				(flt(wo_item_qty) - flt(item.consumed_qty)) /
+			wo_qty_to_produce = flt(work_order_qty) - flt(wo.produced_qty)
-					(flt(work_order_qty) - flt(wo.produced_qty))
+
-			)
+			req_qty_each = (wo_qty_consumed) / (wo_qty_to_produce or 1)
 
 			qty = req_qty_each * flt(self.fg_completed_qty)
 

erpnext/stock/reorder_item.py

@@ -166,7 +166,7 @@ def create_material_request(material_requests):
 				mr.submit()
 				mr_list.append(mr)
 
-			except:
+			except Exception:
 				_log_exception()
 
 	if mr_list:

erpnext/stock/report/cogs_by_item_group/cogs_by_item_group.py

@@ -43,13 +43,13 @@ def validate_filters(filters: Filters) -> None:
 def get_columns() -> Columns:
 	return [
 		{
-			'label': 'Item Group',
+			'label': _('Item Group'),
 			'fieldname': 'item_group',
 			'fieldtype': 'Data',
 			'width': '200'
 		},
 		{
-			'label': 'COGS Debit',
+			'label': _('COGS Debit'),
 			'fieldname': 'cogs_debit',
 			'fieldtype': 'Currency',
 			'width': '200'

erpnext/stock/report/process_loss_report/process_loss_report.py

@@ -2,6 +2,7 @@
 # For license information, please see license.txt
 
 import frappe
+from frappe import _
 from typing import Dict, List, Tuple
 
 Filters = frappe._dict
@@ -24,57 +25,57 @@ def get_data(filters: Filters) -> Data:
 def get_columns() -> Columns:
 	return [
 		{
-			'label': 'Work Order',
+			'label': _('Work Order'),
 			'fieldname': 'name',
 			'fieldtype': 'Link',
 			'options': 'Work Order',
 			'width': '200'
 		},
 		{
-			'label': 'Item',
+			'label': _('Item'),
 			'fieldname': 'production_item',
 			'fieldtype': 'Link',
 			'options': 'Item',
 			'width': '100'
 		},
 		{
-			'label': 'Status',
+			'label': _('Status'),
 			'fieldname': 'status',
 			'fieldtype': 'Data',
 			'width': '100'
 		},
 		{
-			'label': 'Manufactured Qty',
+			'label': _('Manufactured Qty'),
 			'fieldname': 'produced_qty',
 			'fieldtype': 'Float',
 			'width': '150'
 		},
 		{
-			'label': 'Loss Qty',
+			'label': _('Loss Qty'),
 			'fieldname': 'process_loss_qty',
 			'fieldtype': 'Float',
 			'width': '150'
 		},
 		{
-			'label': 'Actual Manufactured Qty',
+			'label': _('Actual Manufactured Qty'),
 			'fieldname': 'actual_produced_qty',
 			'fieldtype': 'Float',
 			'width': '150'
 		},
 		{
-			'label': 'Loss Value',
+			'label': _('Loss Value'),
 			'fieldname': 'total_pl_value',
 			'fieldtype': 'Float',
 			'width': '150'
 		},
 		{
-			'label': 'FG Value',
+			'label': _('FG Value'),
 			'fieldname': 'total_fg_value',
 			'fieldtype': 'Float',
 			'width': '150'
 		},
 		{
-			'label': 'Raw Material Value',
+			'label': _('Raw Material Value'),
 			'fieldname': 'total_rm_value',
 			'fieldtype': 'Float',
 			'width': '150'

erpnext/stock/report/stock_ageing/stock_ageing.py

@@ -4,6 +4,7 @@
 from __future__ import unicode_literals
 import frappe
 from frappe import _
+from operator import itemgetter
 from frappe.utils import date_diff, flt, cint
 from six import iteritems
 from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos
@@ -12,7 +13,7 @@ def execute(filters=None):
 	columns = get_columns(filters)
 	item_details = get_fifo_queue(filters)
 	to_date = filters["to_date"]
-	_func = lambda x: x[1]
+	_func = itemgetter(1)
 
 	data = []
 	for item, item_dict in iteritems(item_details):

erpnext/stock/report/stock_balance/stock_balance.py

@@ -2,6 +2,7 @@
 # License: GNU General Public License v3. See license.txt
 
 from __future__ import unicode_literals
+from operator import itemgetter
 import frappe, erpnext
 from frappe import _
 from frappe.utils import flt, cint, getdate, now, date_diff
@@ -44,7 +45,7 @@ def execute(filters=None):
 	data = []
 	conversion_factors = {}
 
-	_func = lambda x: x[1]
+	_func = itemgetter(1)
 
 	for (company, item, warehouse) in sorted(iwb_map):
 		if item_map.get(item):

erpnext/stock/stock_balance.py

@@ -29,7 +29,7 @@ def repost(only_actual=False, allow_negative_stock=False, allow_zero_rate=False,
 		try:
 			repost_stock(d[0], d[1], allow_zero_rate, only_actual, only_bin, allow_negative_stock)
 			frappe.db.commit()
-		except:
+		except Exception:
 			frappe.db.rollback()
 
 	if allow_negative_stock:
@@ -247,5 +247,5 @@ def reset_serial_no_status_and_warehouse(serial_nos=None):
 
 				sr.via_stock_ledger = True
 				sr.save()
-			except:
+			except Exception:
 				pass

erpnext/stock/stock_ledger.py

@@ -271,13 +271,15 @@ class update_entries_after(object):
 			}
 
 		"""
+		self.data.setdefault(args.warehouse, frappe._dict())
+		warehouse_dict = self.data[args.warehouse]
 		previous_sle = get_previous_sle_of_current_voucher(args)
+		warehouse_dict.previous_sle = previous_sle
 
-		self.data[args.warehouse] = frappe._dict({
+		for key in ("qty_after_transaction", "valuation_rate", "stock_value"):
-			"previous_sle": previous_sle,
+			setattr(warehouse_dict, key, flt(previous_sle.get(key)))
-			"qty_after_transaction": flt(previous_sle.qty_after_transaction),
+
-			"valuation_rate": flt(previous_sle.valuation_rate),
+		warehouse_dict.update({
-			"stock_value": flt(previous_sle.stock_value),
 			"prev_stock_value": previous_sle.stock_value or 0.0,
 			"stock_queue": json.loads(previous_sle.stock_queue or "[]"),
 			"stock_value_difference": 0.0

erpnext/support/doctype/issue/issue.py

@@ -5,10 +5,10 @@ from __future__ import unicode_literals
 import frappe
 import json
 from frappe import _
-from frappe import utils
 from frappe.model.document import Document
-from frappe.utils import cint, now_datetime, getdate, get_weekdays, add_to_date, get_time, get_datetime, time_diff_in_seconds
+from frappe.utils import now_datetime, time_diff_in_seconds, get_datetime, date_diff, get_weekdays, add_to_date, cint, getdate, get_time
-from datetime import datetime, timedelta
+from frappe.core.utils import get_parent_doc
+from datetime import timedelta, datetime
 from frappe.model.mapper import get_mapped_doc
 from frappe.utils.user import is_website_user
 from erpnext.support.doctype.service_level_agreement.service_level_agreement import get_active_service_level_agreement_for
@@ -24,11 +24,9 @@ class Issue(Document):
 
 		if not self.raised_by:
 			self.raised_by = frappe.session.user
-
 		self.change_service_level_agreement_and_priority()
 		self.update_status()
 		self.set_lead_contact(self.raised_by)
-
 		if not self.service_level_agreement:
 			self.reset_sla_fields()
 
@@ -293,10 +291,6 @@ class Issue(Document):
 		self.agreement_status = "Ongoing"
 		self.save()
 
-	def reset_issue_metrics(self):
-		self.db_set("resolution_time", None)
-		self.db_set("user_resolution_time", None)
-
 
 def get_priority(issue):
 	service_level_agreement = frappe.get_doc("Service Level Agreement", issue.service_level_agreement)
@@ -524,5 +518,120 @@ def get_time_in_timedelta(time):
 	"""
 		Converts datetime.time(10, 36, 55, 961454) to datetime.timedelta(seconds=38215)
 	"""
-	import datetime
+	return timedelta(hours=time.hour, minutes=time.minute, seconds=time.second)
-	return datetime.timedelta(hours=time.hour, minutes=time.minute, seconds=time.second)
+
+def set_first_response_time(communication, method):
+	if communication.get('reference_doctype') == "Issue":
+		issue = get_parent_doc(communication)
+		if is_first_response(issue):
+			first_response_time = calculate_first_response_time(issue, get_datetime(issue.first_responded_on))
+			issue.db_set("first_response_time", first_response_time)
+
+def is_first_response(issue):
+	responses = frappe.get_all('Communication', filters = {'reference_name': issue.name, 'sent_or_received': 'Sent'})
+	if len(responses) == 1:
+		return True
+	return False
+
+def calculate_first_response_time(issue, first_responded_on):
+	issue_creation_date = issue.creation
+	issue_creation_time = get_time_in_seconds(issue_creation_date)
+	first_responded_on_in_seconds = get_time_in_seconds(first_responded_on)
+	support_hours = frappe.get_cached_doc("Service Level Agreement", issue.service_level_agreement).support_and_resolution
+
+	if issue_creation_date.day == first_responded_on.day:
+		if is_work_day(issue_creation_date, support_hours):
+			start_time, end_time = get_working_hours(issue_creation_date, support_hours)
+
+			# issue creation and response on the same day during working hours
+			if is_during_working_hours(issue_creation_date, support_hours) and is_during_working_hours(first_responded_on, support_hours):
+				return get_elapsed_time(issue_creation_date, first_responded_on)
+
+			# issue creation is during working hours, but first response was after working hours
+			elif is_during_working_hours(issue_creation_date, support_hours):
+				return get_elapsed_time(issue_creation_time, end_time)
+
+			# issue creation was before working hours but first response is during working hours
+			elif is_during_working_hours(first_responded_on, support_hours):
+				return get_elapsed_time(start_time, first_responded_on_in_seconds)
+
+			# both issue creation and first response were after working hours
+			else:
+				return 1.0		# this should ideally be zero, but it gets reset when the next response is sent if the value is zero
+
+		else:
+			return 1.0
+
+	else:
+		# response on the next day
+		if date_diff(first_responded_on, issue_creation_date) == 1:
+			first_response_time = 0
+		else:
+			first_response_time = calculate_initial_frt(issue_creation_date, date_diff(first_responded_on, issue_creation_date)- 1, support_hours)
+
+		# time taken on day of issue creation
+		if is_work_day(issue_creation_date, support_hours):
+			start_time, end_time = get_working_hours(issue_creation_date, support_hours)
+
+			if is_during_working_hours(issue_creation_date, support_hours):
+				first_response_time += get_elapsed_time(issue_creation_time, end_time)
+			elif is_before_working_hours(issue_creation_date, support_hours):
+				first_response_time += get_elapsed_time(start_time, end_time)
+
+		# time taken on day of first response
+		if is_work_day(first_responded_on, support_hours):
+			start_time, end_time = get_working_hours(first_responded_on, support_hours)
+
+			if is_during_working_hours(first_responded_on, support_hours):
+				first_response_time += get_elapsed_time(start_time, first_responded_on_in_seconds)
+			elif not is_before_working_hours(first_responded_on, support_hours):
+				first_response_time += get_elapsed_time(start_time, end_time)
+
+		if first_response_time:
+			return first_response_time
+		else:
+			return 1.0
+
+def get_time_in_seconds(date):
+	return timedelta(hours=date.hour, minutes=date.minute, seconds=date.second)
+
+def get_working_hours(date, support_hours):
+	if is_work_day(date, support_hours):
+		weekday = frappe.utils.get_weekday(date)
+		for day in support_hours:
+			if day.workday == weekday:
+				return day.start_time, day.end_time
+
+def is_work_day(date, support_hours):
+	weekday = frappe.utils.get_weekday(date)
+	for day in support_hours:
+		if day.workday == weekday:
+			return True
+	return False
+
+def is_during_working_hours(date, support_hours):
+	start_time, end_time = get_working_hours(date, support_hours)
+	time = get_time_in_seconds(date)
+	if time >= start_time and time <= end_time:
+		return True
+	return False
+
+def get_elapsed_time(start_time, end_time):
+	return round(time_diff_in_seconds(end_time, start_time), 2)
+
+def calculate_initial_frt(issue_creation_date, days_in_between, support_hours):
+	initial_frt = 0
+	for i in range(days_in_between):
+		date = issue_creation_date + timedelta(days = (i+1))
+		if is_work_day(date, support_hours):
+			start_time, end_time = get_working_hours(date, support_hours)
+			initial_frt += get_elapsed_time(start_time, end_time)
+
+	return initial_frt
+
+def is_before_working_hours(date, support_hours):
+	start_time, end_time = get_working_hours(date, support_hours)
+	time = get_time_in_seconds(date)
+	if time < start_time:
+		return True
+	return False

erpnext/support/doctype/issue/test_issue.py

@@ -5,69 +5,69 @@ from __future__ import unicode_literals
 import frappe
 import unittest
 from erpnext.support.doctype.service_level_agreement.test_service_level_agreement import create_service_level_agreements_for_issues
-from frappe.utils import now_datetime, get_datetime, flt
+from frappe.core.doctype.user_permission.test_user_permission import create_user
-import datetime
+from frappe.utils import get_datetime, flt
-from datetime import timedelta
 
-class TestIssue(unittest.TestCase):
+class TestSetUp(unittest.TestCase):
 	def setUp(self):
 		frappe.db.sql("delete from `tabService Level Agreement`")
 		frappe.db.set_value("Support Settings", None, "track_service_level_agreement", 1)
 		create_service_level_agreements_for_issues()
 
+class TestIssue(TestSetUp):
 	def test_response_time_and_resolution_time_based_on_different_sla(self):
-		creation = datetime.datetime(2019, 3, 4, 12, 0)
+		creation = get_datetime("2019-03-04 12:00")
 
 		# make issue with customer specific SLA
 		customer = create_customer("_Test Customer", "__Test SLA Customer Group", "__Test SLA Territory")
 		issue = make_issue(creation, "_Test Customer", 1)
 
-		self.assertEqual(issue.response_by, datetime.datetime(2019, 3, 4, 14, 0))
+		self.assertEqual(issue.response_by, get_datetime("2019-03-04 14:00"))
-		self.assertEqual(issue.resolution_by, datetime.datetime(2019, 3, 4, 15, 0))
+		self.assertEqual(issue.resolution_by, get_datetime("2019-03-04 15:00"))
 
 		# make issue with customer_group specific SLA
 		customer = create_customer("__Test Customer", "_Test SLA Customer Group", "__Test SLA Territory")
 		issue = make_issue(creation, "__Test Customer", 2)
 
-		self.assertEqual(issue.response_by, datetime.datetime(2019, 3, 4, 14, 0))
+		self.assertEqual(issue.response_by, get_datetime("2019-03-04 14:00"))
-		self.assertEqual(issue.resolution_by, datetime.datetime(2019, 3, 4, 15, 0))
+		self.assertEqual(issue.resolution_by, get_datetime("2019-03-04 15:00"))
 
 
 		# make issue with territory specific SLA
 		customer = create_customer("___Test Customer", "__Test SLA Customer Group", "_Test SLA Territory")
 		issue = make_issue(creation, "___Test Customer", 3)
 
-		self.assertEqual(issue.response_by, datetime.datetime(2019, 3, 4, 14, 0))
+		self.assertEqual(issue.response_by, get_datetime("2019-03-04 14:00"))
-		self.assertEqual(issue.resolution_by, datetime.datetime(2019, 3, 4, 15, 0))
+		self.assertEqual(issue.resolution_by, get_datetime("2019-03-04 15:00"))
 
 		# make issue with default SLA
 		issue = make_issue(creation=creation, index=4)
 
-		self.assertEqual(issue.response_by, datetime.datetime(2019, 3, 4, 16, 0))
+		self.assertEqual(issue.response_by, get_datetime("2019-03-04 16:00"))
-		self.assertEqual(issue.resolution_by, datetime.datetime(2019, 3, 4, 18, 0))
+		self.assertEqual(issue.resolution_by, get_datetime("2019-03-04 18:00"))
 
 		# make issue with default SLA before working hours
-		creation = datetime.datetime(2019, 3, 4, 7, 0)
+		creation = get_datetime("2019-03-04 7:00")
 		issue = make_issue(creation=creation, index=5)
 
-		self.assertEqual(issue.response_by, datetime.datetime(2019, 3, 4, 14, 0))
+		self.assertEqual(issue.response_by, get_datetime("2019-03-04 14:00"))
-		self.assertEqual(issue.resolution_by, datetime.datetime(2019, 3, 4, 16, 0))
+		self.assertEqual(issue.resolution_by, get_datetime("2019-03-04 16:00"))
 
 		# make issue with default SLA after working hours
-		creation = datetime.datetime(2019, 3, 4, 20, 0)
+		creation = get_datetime("2019-03-04 20:00")
 		issue = make_issue(creation, index=6)
 
-		self.assertEqual(issue.response_by, datetime.datetime(2019, 3, 6, 14, 0))
+		self.assertEqual(issue.response_by, get_datetime("2019-03-06 14:00"))
-		self.assertEqual(issue.resolution_by, datetime.datetime(2019, 3, 6, 16, 0))
+		self.assertEqual(issue.resolution_by, get_datetime("2019-03-06 16:00"))
 
 		# make issue with default SLA next day
-		creation = datetime.datetime(2019, 3, 4, 14, 0)
+		creation = get_datetime("2019-03-04 14:00")
 		issue = make_issue(creation=creation, index=7)
 
-		self.assertEqual(issue.response_by, datetime.datetime(2019, 3, 4, 18, 0))
+		self.assertEqual(issue.response_by, get_datetime("2019-03-04 18:00"))
-		self.assertEqual(issue.resolution_by, datetime.datetime(2019, 3, 6, 12, 0))
+		self.assertEqual(issue.resolution_by, get_datetime("2019-03-06 12:00"))
 
-		frappe.flags.current_time = datetime.datetime(2019, 3, 4, 15, 0)
+		frappe.flags.current_time = get_datetime("2019-03-04 15:00")
 
 		issue.status = 'Closed'
 		issue.save()
@@ -75,21 +75,21 @@ class TestIssue(unittest.TestCase):
 		self.assertEqual(issue.agreement_status, 'Fulfilled')
 
 	def test_issue_metrics(self):
-		creation = datetime.datetime(2020, 3, 4, 4, 0)
+		creation = get_datetime("2020-03-04 4:00")
 
 		issue = make_issue(creation, index=1)
 		create_communication(issue.name, "test@example.com", "Received", creation)
 
-		creation = datetime.datetime(2020, 3, 4, 4, 15)
+		creation = get_datetime("2020-03-04 4:15")
 		create_communication(issue.name, "test@admin.com", "Sent", creation)
 
-		creation = datetime.datetime(2020, 3, 4, 5, 0)
+		creation = get_datetime("2020-03-04 5:00")
 		create_communication(issue.name, "test@example.com", "Received", creation)
 
-		creation = datetime.datetime(2020, 3, 4, 5, 5)
+		creation = get_datetime("2020-03-04 5:05")
 		create_communication(issue.name, "test@admin.com", "Sent", creation)
 
-		frappe.flags.current_time = datetime.datetime(2020, 3, 4, 5, 5)
+		frappe.flags.current_time = get_datetime("2020-03-04 5:05")
 		issue.reload()
 		issue.status = 'Closed'
 		issue.save()
@@ -99,33 +99,33 @@ class TestIssue(unittest.TestCase):
 		self.assertEqual(issue.user_resolution_time, 1200)
 
 	def test_hold_time_on_replied(self):
-		creation = datetime.datetime(2020, 3, 4, 4, 0)
+		creation = get_datetime("2020-03-04 4:00")
 
 		issue = make_issue(creation, index=1)
 		create_communication(issue.name, "test@example.com", "Received", creation)
 
-		creation = datetime.datetime(2020, 3, 4, 4, 15)
+		creation = get_datetime("2020-03-04 4:15")
 		create_communication(issue.name, "test@admin.com", "Sent", creation)
 
-		frappe.flags.current_time = datetime.datetime(2020, 3, 4, 4, 15)
+		frappe.flags.current_time = get_datetime("2020-03-04 4:15")
 		issue.reload()
 		issue.status = 'Replied'
 		issue.save()
 
 		self.assertEqual(issue.on_hold_since, frappe.flags.current_time)
 
-		creation = datetime.datetime(2020, 3, 4, 5, 0)
+		creation = get_datetime("2020-03-04 5:00")
-		frappe.flags.current_time = datetime.datetime(2020, 3, 4, 5, 0)
+		frappe.flags.current_time = get_datetime("2020-03-04 5:00")
 		create_communication(issue.name, "test@example.com", "Received", creation)
 
 		issue.reload()
 		self.assertEqual(flt(issue.total_hold_time, 2), 2700)
-		self.assertEqual(issue.resolution_by, datetime.datetime(2020, 3, 4, 16, 45))
+		self.assertEqual(issue.resolution_by, get_datetime("2020-03-04 16:45"))
 
-		creation = datetime.datetime(2020, 3, 4, 5, 5)
+		creation = get_datetime("2020-03-04 5:05")
 		create_communication(issue.name, "test@admin.com", "Sent", creation)
 
-		frappe.flags.current_time = datetime.datetime(2020, 3, 4, 5, 5)
+		frappe.flags.current_time = get_datetime("2020-03-04 5:05")
 		issue.reload()
 		issue.status = 'Closed'
 		issue.save()
@@ -133,6 +133,223 @@ class TestIssue(unittest.TestCase):
 		issue.reload()
 		self.assertEqual(flt(issue.total_hold_time, 2), 2700)
 
+class TestFirstResponseTime(TestSetUp):
+	# working hours used in all cases: Mon-Fri, 10am to 6pm
+	# all dates are in the mm-dd-yyyy format
+
+	# issue creation and first response are on the same day
+	def test_first_response_time_case1(self):
+		"""
+			Test frt when issue creation and first response are during working hours on the same day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 11:00"), get_datetime("06-28-2021 12:00"))
+		self.assertEqual(issue.first_response_time, 3600.0)
+
+	def test_first_response_time_case2(self):
+		"""
+			Test frt when issue creation was during working hours, but first response is sent after working hours on the same day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 12:00"), get_datetime("06-28-2021 20:00"))
+		self.assertEqual(issue.first_response_time, 21600.0)
+
+	def test_first_response_time_case3(self):
+		"""
+			Test frt when issue creation was before working hours but first response is sent during working hours on the same day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 6:00"), get_datetime("06-28-2021 12:00"))
+		self.assertEqual(issue.first_response_time, 7200.0)
+
+	def test_first_response_time_case4(self):
+		"""
+			Test frt when both issue creation and first response were after working hours on the same day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 19:00"), get_datetime("06-28-2021 20:00"))
+		self.assertEqual(issue.first_response_time, 1.0)
+
+	def test_first_response_time_case5(self):
+		"""
+			Test frt when both issue creation and first response are on the same day, but it's not a work day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-27-2021 10:00"), get_datetime("06-27-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 1.0)
+
+	# issue creation and first response are on consecutive days
+	def test_first_response_time_case6(self):
+		"""
+			Test frt when the issue was created before working hours and the first response is also sent before working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 6:00"), get_datetime("06-29-2021 6:00"))
+		self.assertEqual(issue.first_response_time, 28800.0)
+
+	def test_first_response_time_case7(self):
+		"""
+			Test frt when the issue was created before working hours and the first response is sent during working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 6:00"), get_datetime("06-29-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 32400.0)
+
+	def test_first_response_time_case8(self):
+		"""
+			Test frt when the issue was created before working hours and the first response is sent after working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 6:00"), get_datetime("06-29-2021 20:00"))
+		self.assertEqual(issue.first_response_time, 57600.0)
+
+	def test_first_response_time_case9(self):
+		"""
+			Test frt when the issue was created before working hours and the first response is sent on the next day, which is not a work day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-25-2021 6:00"), get_datetime("06-26-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 28800.0)
+
+	def test_first_response_time_case10(self):
+		"""
+			Test frt when the issue was created during working hours and the first response is sent before working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 12:00"), get_datetime("06-29-2021 6:00"))
+		self.assertEqual(issue.first_response_time, 21600.0)
+
+	def test_first_response_time_case11(self):
+		"""
+			Test frt when the issue was created during working hours and the first response is also sent during working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 12:00"), get_datetime("06-29-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 25200.0)
+
+	def test_first_response_time_case12(self):
+		"""
+			Test frt when the issue was created during working hours and the first response is sent after working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 12:00"), get_datetime("06-29-2021 20:00"))
+		self.assertEqual(issue.first_response_time, 50400.0)
+
+	def test_first_response_time_case13(self):
+		"""
+			Test frt when the issue was created during working hours and the first response is sent on the next day, which is not a work day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-25-2021 12:00"), get_datetime("06-26-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 21600.0)
+
+	def test_first_response_time_case14(self):
+		"""
+			Test frt when the issue was created after working hours and the first response is sent before working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 20:00"), get_datetime("06-29-2021 6:00"))
+		self.assertEqual(issue.first_response_time, 1.0)
+
+	def test_first_response_time_case15(self):
+		"""
+			Test frt when the issue was created after working hours and the first response is sent during working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 20:00"), get_datetime("06-29-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 3600.0)
+
+	def test_first_response_time_case16(self):
+		"""
+			Test frt when the issue was created after working hours and the first response is also sent after working hours, but on the next day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 20:00"), get_datetime("06-29-2021 20:00"))
+		self.assertEqual(issue.first_response_time, 28800.0)
+
+	def test_first_response_time_case17(self):
+		"""
+			Test frt when the issue was created after working hours and the first response is sent on the next day, which is not a work day.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-25-2021 20:00"), get_datetime("06-26-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 1.0)
+
+	# issue creation and first response are a few days apart
+	def test_first_response_time_case18(self):
+		"""
+			Test frt when the issue was created before working hours and the first response is also sent before working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 6:00"), get_datetime("07-01-2021 6:00"))
+		self.assertEqual(issue.first_response_time, 86400.0)
+
+	def test_first_response_time_case19(self):
+		"""
+			Test frt when the issue was created before working hours and the first response is sent during working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 6:00"), get_datetime("07-01-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 90000.0)
+
+	def test_first_response_time_case20(self):
+		"""
+			Test frt when the issue was created before working hours and the first response is sent after working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 6:00"), get_datetime("07-01-2021 20:00"))
+		self.assertEqual(issue.first_response_time, 115200.0)
+
+	def test_first_response_time_case21(self):
+		"""
+			Test frt when the issue was created before working hours and the first response is sent after a few days, on a holiday.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-25-2021 6:00"), get_datetime("06-27-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 28800.0)
+
+	def test_first_response_time_case22(self):
+		"""
+			Test frt when the issue was created during working hours and the first response is sent before working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 12:00"), get_datetime("07-01-2021 6:00"))
+		self.assertEqual(issue.first_response_time, 79200.0)
+
+	def test_first_response_time_case23(self):
+		"""
+			Test frt when the issue was created during working hours and the first response is also sent during working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 12:00"), get_datetime("07-01-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 82800.0)
+
+	def test_first_response_time_case24(self):
+		"""
+			Test frt when the issue was created during working hours and the first response is sent after working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 12:00"), get_datetime("07-01-2021 20:00"))
+		self.assertEqual(issue.first_response_time, 108000.0)
+
+	def test_first_response_time_case25(self):
+		"""
+			Test frt when the issue was created during working hours and the first response is sent after a few days, on a holiday.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-25-2021 12:00"), get_datetime("06-27-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 21600.0)
+
+	def test_first_response_time_case26(self):
+		"""
+			Test frt when the issue was created after working hours and the first response is sent before working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 20:00"), get_datetime("07-01-2021 6:00"))
+		self.assertEqual(issue.first_response_time, 57600.0)
+
+	def test_first_response_time_case27(self):
+		"""
+			Test frt when the issue was created after working hours and the first response is sent during working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 20:00"), get_datetime("07-01-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 61200.0)
+
+	def test_first_response_time_case28(self):
+		"""
+			Test frt when the issue was created after working hours and the first response is also sent after working hours, but after a few days.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-28-2021 20:00"), get_datetime("07-01-2021 20:00"))
+		self.assertEqual(issue.first_response_time, 86400.0)
+
+	def test_first_response_time_case29(self):
+		"""
+			Test frt when the issue was created after working hours and the first response is sent after a few days, on a holiday.
+		"""
+		issue = create_issue_and_communication(get_datetime("06-25-2021 20:00"), get_datetime("06-27-2021 11:00"))
+		self.assertEqual(issue.first_response_time, 1.0)
+
+def create_issue_and_communication(issue_creation, first_responded_on):
+	issue = make_issue(issue_creation, index=1)
+	sender = create_user("test@admin.com")
+	create_communication(issue.name, sender.email, "Sent", first_responded_on)
+	issue.reload()
+
+	return issue
 
 def make_issue(creation=None, customer=None, index=0, priority=None, issue_type=None):
 	issue = frappe.get_doc({
@@ -185,7 +402,7 @@ def create_territory(territory):
 
 
 def create_communication(reference_name, sender, sent_or_received, creation):
-	issue = frappe.get_doc({
+	communication = frappe.get_doc({
 		"doctype": "Communication",
 		"communication_type": "Communication",
 		"communication_medium": "Email",
@@ -199,4 +416,4 @@ def create_communication(reference_name, sender, sent_or_received, creation):
 		"creation": creation,
 		"reference_name": reference_name
 	})
-	issue.save()
+	communication.save()

erpnext/support/doctype/service_level_agreement/test_service_level_agreement.py

@@ -145,16 +145,6 @@ def create_service_level_agreement(default_service_level_agreement, holiday_list
 				"workday": "Friday",
 				"start_time": "10:00:00",
 				"end_time": "18:00:00",
-			},
-			{
-				"workday": "Saturday",
-				"start_time": "10:00:00",
-				"end_time": "18:00:00",
-			},
-			{
-				"workday": "Sunday",
-				"start_time": "10:00:00",
-				"end_time": "18:00:00",
 			}
 		]
 	})

erpnext/utilities/hierarchy_chart.py

@@ -6,17 +6,21 @@ import frappe
 from frappe import _
 
 @frappe.whitelist()
-def get_all_nodes(parent, parent_name, method, company):
+def get_all_nodes(method, company):
 	'''Recursively gets all data from nodes'''
 	method = frappe.get_attr(method)
 
 	if method not in frappe.whitelisted:
 		frappe.throw(_('Not Permitted'), frappe.PermissionError)
 
-	data = method(parent, company)
+	root_nodes = method(company=company)
-	result = [dict(parent=parent, parent_name=parent_name, data=data)]
+	result = []
+	nodes_to_expand = []
 
-	nodes_to_expand = [{'id': d.get('id'), 'name': d.get('name')} for d in data if d.get('expandable')]
+	for root in root_nodes:
+		data = method(root.id, company)
+		result.append(dict(parent=root.id, parent_name=root.name, data=data))
+		nodes_to_expand.extend([{'id': d.get('id'), 'name': d.get('name')} for d in data if d.get('expandable')])
 
 	while nodes_to_expand:
 		parent = nodes_to_expand.pop(0)