From dcfc3d7d1278d90e0a9154c9d7d7d2dcd2b57bc4 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 4 Mar 2021 19:29:55 +0100 Subject: [PATCH 01/31] fix: remove redundant calls to create_sales_tax --- erpnext/regional/saudi_arabia/setup.py | 5 +---- erpnext/regional/united_arab_emirates/setup.py | 4 +--- erpnext/setup/setup_wizard/utils.py | 1 - 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/erpnext/regional/saudi_arabia/setup.py b/erpnext/regional/saudi_arabia/setup.py index d9ac6cb0f6..9b3677d2c6 100644 --- a/erpnext/regional/saudi_arabia/setup.py +++ b/erpnext/regional/saudi_arabia/setup.py @@ -4,11 +4,8 @@ from __future__ import unicode_literals from erpnext.regional.united_arab_emirates.setup import make_custom_fields, add_print_formats -from erpnext.setup.setup_wizard.operations.taxes_setup import create_sales_tax + def setup(company=None, patch=True): make_custom_fields() add_print_formats() - - if company: - create_sales_tax(company) \ No newline at end of file diff --git a/erpnext/regional/united_arab_emirates/setup.py b/erpnext/regional/united_arab_emirates/setup.py index 776a82c730..d5a29fc200 100644 --- a/erpnext/regional/united_arab_emirates/setup.py +++ b/erpnext/regional/united_arab_emirates/setup.py @@ -6,15 +6,13 @@ from __future__ import unicode_literals import frappe, os, json from frappe.custom.doctype.custom_field.custom_field import create_custom_fields from frappe.permissions import add_permission, update_permission_property -from erpnext.setup.setup_wizard.operations.taxes_setup import create_sales_tax + def setup(company=None, patch=True): make_custom_fields() add_print_formats() add_custom_roles_for_reports() add_permissions() - if company: - create_sales_tax(company) def make_custom_fields(): is_zero_rated = dict(fieldname='is_zero_rated', label='Is Zero Rated', diff --git a/erpnext/setup/setup_wizard/utils.py b/erpnext/setup/setup_wizard/utils.py index e82bc96d93..4223f000a6 100644 --- a/erpnext/setup/setup_wizard/utils.py +++ b/erpnext/setup/setup_wizard/utils.py @@ -9,5 +9,4 @@ def complete(): 'data', 'test_mfg.json'), 'r') as f: data = json.loads(f.read()) - #setup_wizard.create_sales_tax(data) setup_complete(data) From 25afad3dc1d75031ba09b405ff8b12de5f0d8007 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 4 Mar 2021 21:11:31 +0100 Subject: [PATCH 02/31] refactor: extend taxes and charges setup Add option to specify taxes and charges template depending on the CoA used. Differentiate between purchase, sales and item taxes. Maintain flexibility by using wildcards. --- erpnext/setup/doctype/company/company.py | 7 +- .../setup_wizard/data/country_wise_tax.json | 307 ++++++++++++++-- .../setup_wizard/operations/taxes_setup.py | 330 ++++++++++++------ 3 files changed, 513 insertions(+), 131 deletions(-) diff --git a/erpnext/setup/doctype/company/company.py b/erpnext/setup/doctype/company/company.py index 819ba78e66..d3021bafea 100644 --- a/erpnext/setup/doctype/company/company.py +++ b/erpnext/setup/doctype/company/company.py @@ -17,6 +17,7 @@ from frappe.utils.nestedset import NestedSet from past.builtins import cmp import functools from erpnext.accounts.doctype.account.account import get_account_currency +from erpnext.setup.setup_wizard.operations.taxes_setup import setup_taxes_and_charges class Company(NestedSet): nsm_parent_field = 'parent_company' @@ -67,11 +68,7 @@ class Company(NestedSet): frappe.throw(_("Abbreviation already used for another company")) def create_default_tax_template(self): - from erpnext.setup.setup_wizard.operations.taxes_setup import create_sales_tax - create_sales_tax({ - 'country': self.country, - 'company_name': self.name - }) + setup_taxes_and_charges(self.name, self.country) def validate_default_accounts(self): accounts = [ diff --git a/erpnext/setup/setup_wizard/data/country_wise_tax.json b/erpnext/setup/setup_wizard/data/country_wise_tax.json index beddaeed79..9ccbdb965b 100644 --- a/erpnext/setup/setup_wizard/data/country_wise_tax.json +++ b/erpnext/setup/setup_wizard/data/country_wise_tax.json @@ -481,14 +481,230 @@ }, "Germany": { - "Germany VAT 19%": { - "account_name": "VAT 19%", - "tax_rate": 19.00, - "default": 1 - }, - "Germany VAT 7%": { - "account_name": "VAT 7%", - "tax_rate": 7.00 + "chart_of_accounts": { + "SKR04 mit Kontonummern": { + "sales_tax_templates": [ + { + "title": "Umsatzsteuer 19%", + "is_default": 1, + "accounts": [ + { + "type": "On Net Total", + "account_name": "Umsatzsteuer 19%", + "account_number": "3806", + "rate": 19.00 + } + ] + }, + { + "title": "Umsatzsteuer 7%", + "accounts": [ + { + "type": "On Net Total", + "account_name": "Umsatzsteuer 7%", + "account_number": "3801", + "tax_rate": 7.00 + } + ] + } + ], + "purchase_tax_templates": [ + { + "title": "Abziehbare Vorsteuer 19%", + "is_default": 1, + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer 19%", + "account_number": "1406", + "root_type": "Asset", + "tax_rate": 19.00 + } + ] + }, + { + "title": "Abziehbare Vorsteuer 7%", + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer 7%", + "account_number": "1401", + "root_type": "Asset", + "tax_rate": 7.00 + } + ] + }, + { + "title": "Innergemeinschaftlicher Erwerb 19% Umsatzsteuer und 19% Vorsteuer", + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer nach § 13b UStG 19%", + "account_number": "1407", + "root_type": "Asset", + "tax_rate": 19.00, + "add_deduct_tax": "Add" + }, + { + "account_name": "Umsatzsteuer nach § 13b UStG 19%", + "account_number": "3837", + "root_type": "Liability", + "tax_rate": 19.00, + "add_deduct_tax": "Deduct" + } + ] + } + ] + }, + "SKR03 mit Kontonummern": { + "sales_tax_templates": [ + { + "title": "Umsatzsteuer 19%", + "is_default": 1, + "accounts": [ + { + "type": "On Net Total", + "account_name": "Umsatzsteuer 19%", + "account_number": "1776", + "rate": 19.00 + } + ] + }, + { + "title": "Umsatzsteuer 7%", + "accounts": [ + { + "type": "On Net Total", + "account_name": "Umsatzsteuer 7%", + "account_number": "1771", + "tax_rate": 7.00 + } + ] + } + ], + "purchase_tax_templates": [ + { + "title": "Abziehbare Vorsteuer 19%", + "is_default": 1, + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer 19%", + "account_number": "1576", + "root_type": "Asset", + "tax_rate": 19.00 + } + ] + }, + { + "title": "Abziehbare Vorsteuer 7%", + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer 7%", + "account_number": "1571", + "root_type": "Asset", + "tax_rate": 7.00 + } + ] + } + ] + }, + "Standard with Numbers": { + "sales_tax_templates": [ + { + "title": "Umsatzsteuer 19%", + "is_default": 1, + "accounts": [ + { + "type": "On Net Total", + "account_name": "Umsatzsteuer 19%", + "account_number": "2301", + "rate": 19.00 + } + ] + }, + { + "title": "Umsatzsteuer 7%", + "accounts": [ + { + "type": "On Net Total", + "account_name": "Umsatzsteuer 7%", + "account_number": "2302", + "tax_rate": 7.00 + } + ] + } + ], + "purchase_tax_templates": [ + { + "title": "Abziehbare Vorsteuer 19%", + "is_default": 1, + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer 19%", + "account_number": "1501", + "root_type": "Asset", + "tax_rate": 19.00 + } + ] + }, + { + "title": "Abziehbare Vorsteuer 7%", + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer 7%", + "account_number": "1502", + "root_type": "Asset", + "tax_rate": 7.00 + } + ] + } + ] + }, + "*": { + "sales_tax_templates": [ + { + "title": "Umsatzsteuer 19%", + "is_default": 1, + "accounts": [ + { + "type": "On Net Total", + "account_name": "Umsatzsteuer 19%", + "rate": 19.00 + } + ] + }, + { + "title": "Umsatzsteuer 7%", + "accounts": [ + { + "type": "On Net Total", + "account_name": "Umsatzsteuer 7%", + "tax_rate": 7.00 + } + ] + } + ], + "purchase_tax_templates": [ + { + "title": "Abziehbare Vorsteuer 19%", + "is_default": 1, + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer 19%", + "root_type": "Asset", + "tax_rate": 19.00 + } + ] + }, + { + "title": "Abziehbare Vorsteuer 7%", + "accounts": [ + { + "account_name": "Abziehbare Vorsteuer 7%", + "root_type": "Asset", + "tax_rate": 7.00 + } + ] + } + ] + } } }, @@ -580,26 +796,61 @@ }, "India": { - "In State GST": { - "account_name": ["SGST", "CGST"], - "tax_rate": [9.00, 9.00], - "default": 1 - }, - "Out of State GST": { - "account_name": "IGST", - "tax_rate": 18.00 - }, - "VAT 5%": { - "account_name": "VAT 5%", - "tax_rate": 5.00 - }, - "VAT 4%": { - "account_name": "VAT 4%", - "tax_rate": 4.00 - }, - "VAT 14%": { - "account_name": "VAT 14%", - "tax_rate": 14.00 + "chart_of_accounts": { + "*": { + "*": [ + { + "title": "In State GST", + "is_default": 1, + "accounts": [ + { + "account_name": "SGST", + "tax_rate": 9.00 + }, + { + "account_name": "CGST", + "tax_rate": 9.00 + } + ] + }, + { + "title": "Out of State GST", + "accounts": [ + { + "account_name": "IGST", + "tax_rate": 18.00 + } + ] + }, + { + "title": "VAT 5%", + "accounts": [ + { + "account_name": "VAT 5%", + "tax_rate": 5.00 + } + ] + }, + { + "title": "VAT 4%", + "accounts": [ + { + "account_name": "VAT 4%", + "tax_rate": 4.00 + } + ] + }, + { + "title": "VAT 14%", + "accounts": [ + { + "account_name": "VAT 14%", + "tax_rate": 14.00 + } + ] + } + ] + } } }, diff --git a/erpnext/setup/setup_wizard/operations/taxes_setup.py b/erpnext/setup/setup_wizard/operations/taxes_setup.py index c3c1593c04..81506c4352 100644 --- a/erpnext/setup/setup_wizard/operations/taxes_setup.py +++ b/erpnext/setup/setup_wizard/operations/taxes_setup.py @@ -1,123 +1,257 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt from __future__ import unicode_literals -import frappe, copy, os, json -from frappe.utils import flt -from erpnext.accounts.doctype.account.account import RootNotEditable -def create_sales_tax(args): - country_wise_tax = get_country_wise_tax(args.get("country")) - if country_wise_tax and len(country_wise_tax) > 0: - for sales_tax, tax_data in country_wise_tax.items(): - make_tax_account_and_template( - args.get("company_name"), - tax_data.get('account_name'), - tax_data.get('tax_rate'), sales_tax) +import os +import json -def make_tax_account_and_template(company, account_name, tax_rate, template_name=None): - if not isinstance(account_name, (list, tuple)): - account_name = [account_name] - tax_rate = [tax_rate] +import frappe +from frappe import _ - accounts = [] - for i, name in enumerate(account_name): - tax_account = make_tax_account(company, account_name[i], tax_rate[i]) - if tax_account: - accounts.append(tax_account) - try: - if accounts: - make_sales_and_purchase_tax_templates(accounts, template_name) - make_item_tax_templates(accounts, template_name) - except frappe.NameError: - if frappe.message_log: frappe.message_log.pop() - except RootNotEditable: - pass +def setup_taxes_and_charges(company_name: str, country: str): + file_path = os.path.join(os.path.dirname(__file__), '..', 'data', 'country_wise_tax.json') + with open(file_path, 'r') as json_file: + tax_data = json.load(json_file) -def make_tax_account(company, account_name, tax_rate): - tax_group = get_tax_account_group(company) - if tax_group: - try: - return frappe.get_doc({ - "doctype":"Account", - "company": company, - "parent_account": tax_group, - "account_name": account_name, - "is_group": 0, - "report_type": "Balance Sheet", - "root_type": "Liability", - "account_type": "Tax", - "tax_rate": flt(tax_rate) if tax_rate else None - }).insert(ignore_permissions=True, ignore_mandatory=True) - except frappe.NameError: - if frappe.message_log: frappe.message_log.pop() - abbr = frappe.get_cached_value('Company', company, 'abbr') - account = '{0} - {1}'.format(account_name, abbr) - return frappe.get_doc('Account', account) + country_wise_tax = tax_data.get(country) -def make_sales_and_purchase_tax_templates(accounts, template_name=None): - if not template_name: - template_name = accounts[0].name + if country_wise_tax: + if 'chart_of_accounts' in country_wise_tax: + from_detailed_data(company_name, country_wise_tax.get('chart_of_accounts')) + else: + from_simple_data(company_name, country_wise_tax) - sales_tax_template = { - "doctype": "Sales Taxes and Charges Template", - "title": template_name, - "company": accounts[0].company, - 'taxes': [] + +def from_detailed_data(company_name, data): + """ + Create Taxes and Charges Templates from detailed data like this: + + { + "chart_of_accounts": { + coa_name: { + "sales_tax_templates": [ + { + 'title': '', + 'is_default': 1, + 'accounts': [ + { + 'account_name': '', + 'account_number': '', + 'root_type': '', + } + ] + } + ], + "purchase_tax_templates": [ ... ], + "item_tax_templates": [ ... ], + "*": [ ... ] + } + } } + """ + coa_name = frappe.db.get_value('Company', company_name, 'chart_of_accounts') + tax_templates = data.get(coa_name) or data.get('*') + sales_tax_templates = tax_templates.get('sales_tax_templates') or tax_templates.get('*') + purchase_tax_templates = tax_templates.get('purchase_tax_templates') or tax_templates.get('*') + item_tax_templates = tax_templates.get('item_tax_templates') or tax_templates.get('*') + + if sales_tax_templates: + for template in sales_tax_templates: + make_tax_template(company_name, 'Sales Taxes and Charges Template', template) + + if purchase_tax_templates: + for template in purchase_tax_templates: + make_tax_template(company_name, 'Purchase Taxes and Charges Template', template) + + if item_tax_templates: + for template in item_tax_templates: + make_item_tax_template(company_name, template) + + +def from_simple_data(company_name, data): + """ + Create Taxes and Charges Templates from simple data like this: + + "Austria Tax": { + "account_name": "VAT", + "tax_rate": 20.00 + } + """ + for template_name, tax_data in data.items(): + template = { + 'title': template_name, + 'is_default': tax_data.get('default'), + 'accounts': [ + { + 'account_name': tax_data.get('account_name'), + 'tax_rate': tax_data.get('tax_rate') + } + ] + } + make_tax_template(company_name, 'Sales Taxes and Charges Template', template) + make_tax_template(company_name, 'Purchase Taxes and Charges Template', template) + make_item_tax_template(company_name, template) + + +def make_tax_template(company_name, doctype, template): + if frappe.db.exists(doctype, {'title': template.get('title'), 'company': company_name}): + return + + accounts = get_or_create_accounts(company_name, template.get('accounts')) + + # Get all fields of the Taxes and Charges Template + tax_template = {'doctype': doctype} + tax_template_fields = frappe.get_meta(doctype).fields + tax_template_fieldnames = [field.fieldname for field in tax_template_fields] + + # Get all fields of the taxes child table + table_doctype = [field.options for field in tax_template_fields if field.fieldname=='taxes'][0] + table_fields = frappe.get_meta(table_doctype).fields + table_field_names = [field.fieldname for field in table_fields] + + # Check if field exists as a key in the import data and, if yes, set the + # value accordingly + for field in tax_template_fieldnames: + if field in template: + tax_template[field] = template.get(field) + + # However, company always fixed and taxes table must be empty to start with + tax_template['company'] = company_name + tax_template['taxes'] = [] for account in accounts: - sales_tax_template['taxes'].append({ - "category": "Total", - "charge_type": "On Net Total", - "account_head": account.name, - "description": "{0} @ {1}".format(account.account_name, account.tax_rate), - "rate": account.tax_rate - }) - # Sales - frappe.get_doc(copy.deepcopy(sales_tax_template)).insert(ignore_permissions=True) + row = { + 'category': 'Total', + 'charge_type': 'On Net Total', + 'account_head': account.get('name'), + 'description': '{0} @ {1}'.format(account.get('account_name'), account.get('tax_rate')), + 'rate': account.get('tax_rate') + } + # Check if field exists as a key in the import data and, if yes, set the + # value accordingly + for field in table_field_names: + if field in account: + row[field] = account.get(field) - # Purchase - purchase_tax_template = copy.deepcopy(sales_tax_template) - purchase_tax_template["doctype"] = "Purchase Taxes and Charges Template" + tax_template['taxes'].append(row) - doc = frappe.get_doc(purchase_tax_template) - doc.insert(ignore_permissions=True) + return frappe.get_doc(tax_template).insert(ignore_permissions=True) -def make_item_tax_templates(accounts, template_name=None): - if not template_name: - template_name = accounts[0].name + +def make_item_tax_template(company_name, template): + """Create an Item Tax Template. + + This requires a separate method because Item Tax Template is structured + differently from Sales and Purchase Tax Templates. + """ + doctype = 'Item Tax Template' + if frappe.db.exists(doctype, {'title': template.get('title'), 'company': company_name}): + return + + accounts = get_or_create_accounts(company_name, template.get('accounts')) item_tax_template = { - "doctype": "Item Tax Template", - "title": template_name, - "company": accounts[0].company, - 'taxes': [] + 'doctype': doctype, + 'title': template.get('title'), + 'company': company_name, + 'taxes': [{ + 'tax_type': account.get('name'), + 'tax_rate': account.get('tax_rate') + } for account in accounts] } + return frappe.get_doc(item_tax_template).insert(ignore_permissions=True) - for account in accounts: - item_tax_template['taxes'].append({ - "tax_type": account.name, - "tax_rate": account.tax_rate - }) - # Items - frappe.get_doc(copy.deepcopy(item_tax_template)).insert(ignore_permissions=True) +def get_or_create_accounts(company: str, account_data: list): + for account in account_data: + if 'creation' in account: + # Hack to check if account already contains a real Account doc + # or just the attibutes from country_wise_tax.json + continue -def get_tax_account_group(company): - tax_group = frappe.db.get_value("Account", - {"account_name": "Duties and Taxes", "is_group": 1, "company": company}) - if not tax_group: - tax_group = frappe.db.get_value("Account", {"is_group": 1, "root_type": "Liability", - "account_type": "Tax", "company": company}) + # tax_rate should survive the following lines because it might not be + # specified in an existing account or different rates might get booked + # onto the same account. + tax_rate = account.get('tax_rate') + doc = get_or_create_account(company, account) + account.update(doc.as_dict()) + account['tax_rate'] = tax_rate + + return account_data + + +def get_or_create_account(company, account_data): + """ + Check if account already exists. If not, create it. + Return a tax account or None. + """ + root_type = account_data.get('root_type', 'Liability') + account_name = account_data.get('account_name') + account_number = account_data.get('account_number') + + existing_accounts = frappe.get_list('Account', + filters={ + 'company': company, + 'root_type': root_type + }, + or_filters={ + 'account_name': account_name, + 'account_number': account_number + } + ) + + if existing_accounts: + return frappe.get_doc('Account', existing_accounts[0].name) + + tax_group = get_or_create_tax_account_group(company, root_type) + full_account_data = { + 'doctype': 'Account', + 'account_name': account_name, + 'account_number': account_number, + 'tax_rate': account_data.get('tax_rate'), + 'company': company, + 'parent_account': tax_group, + 'is_group': 0, + 'report_type': 'Balance Sheet', + 'root_type': root_type, + 'account_type': 'Tax' + } + return frappe.get_doc(full_account_data).insert(ignore_permissions=True, ignore_mandatory=True) + + +def get_or_create_tax_account_group(company, root_type): + tax_group = frappe.db.get_value('Account', { + 'is_group': 1, + 'root_type': root_type, + 'account_type': 'Tax', + 'company': company + }) + + if tax_group: + return tax_group + + root = frappe.get_list('Account', { + 'is_group': 1, + 'root_type': root_type, + 'company': company, + 'report_type': 'Balance Sheet', + 'parent_account': ('is', 'not set') + }, limit=1)[0].name + + doc = frappe.get_doc({ + 'doctype': 'Account', + 'company': company, + 'is_group': 1, + 'report_type': 'Balance Sheet', + 'root_type': root_type, + 'account_type': 'Tax', + 'account_name': _('Duties and Taxes') if root_type == 'Liability' else _('Tax Assets'), + 'parent_account': root + }).insert(ignore_permissions=True) + + tax_group = doc.name return tax_group - -def get_country_wise_tax(country): - data = {} - with open (os.path.join(os.path.dirname(__file__), "..", "data", "country_wise_tax.json")) as countrywise_tax: - data = json.load(countrywise_tax).get(country) - - return data From ebd1d08e55b8146e652096d2c591754f8d676504 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 8 Mar 2021 19:53:50 +0100 Subject: [PATCH 03/31] refactor: taxes setup Better structure of input data. --- .../setup_wizard/data/country_wise_tax.json | 310 ++++++++++++------ .../setup_wizard/operations/taxes_setup.py | 279 +++++++--------- 2 files changed, 334 insertions(+), 255 deletions(-) diff --git a/erpnext/setup/setup_wizard/data/country_wise_tax.json b/erpnext/setup/setup_wizard/data/country_wise_tax.json index 9ccbdb965b..6305442ef2 100644 --- a/erpnext/setup/setup_wizard/data/country_wise_tax.json +++ b/erpnext/setup/setup_wizard/data/country_wise_tax.json @@ -487,23 +487,25 @@ { "title": "Umsatzsteuer 19%", "is_default": 1, - "accounts": [ + "taxes": [ { - "type": "On Net Total", - "account_name": "Umsatzsteuer 19%", - "account_number": "3806", - "rate": 19.00 + "account_head": { + "account_name": "Umsatzsteuer 19%", + "account_number": "3806", + "tax_rate": 19.00 + } } ] }, { "title": "Umsatzsteuer 7%", - "accounts": [ + "taxes": [ { - "type": "On Net Total", - "account_name": "Umsatzsteuer 7%", - "account_number": "3801", - "tax_rate": 7.00 + "account_head": { + "account_name": "Umsatzsteuer 7%", + "account_number": "3801", + "tax_rate": 7.00 + } } ] } @@ -512,41 +514,49 @@ { "title": "Abziehbare Vorsteuer 19%", "is_default": 1, - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer 19%", - "account_number": "1406", - "root_type": "Asset", - "tax_rate": 19.00 + "account_head": { + "account_name": "Abziehbare Vorsteuer 19%", + "account_number": "1406", + "root_type": "Asset", + "tax_rate": 19.00 + } } ] }, { "title": "Abziehbare Vorsteuer 7%", - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer 7%", - "account_number": "1401", - "root_type": "Asset", - "tax_rate": 7.00 + "account_head": { + "account_name": "Abziehbare Vorsteuer 7%", + "account_number": "1401", + "root_type": "Asset", + "tax_rate": 7.00 + } } ] }, { "title": "Innergemeinschaftlicher Erwerb 19% Umsatzsteuer und 19% Vorsteuer", - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer nach § 13b UStG 19%", - "account_number": "1407", - "root_type": "Asset", - "tax_rate": 19.00, + "account_head": { + "account_name": "Abziehbare Vorsteuer nach § 13b UStG 19%", + "account_number": "1407", + "root_type": "Asset", + "tax_rate": 19.00 + }, "add_deduct_tax": "Add" }, { - "account_name": "Umsatzsteuer nach § 13b UStG 19%", - "account_number": "3837", - "root_type": "Liability", - "tax_rate": 19.00, + "account_head": { + "account_name": "Umsatzsteuer nach § 13b UStG 19%", + "account_number": "3837", + "root_type": "Liability", + "tax_rate": 19.00 + }, "add_deduct_tax": "Deduct" } ] @@ -558,23 +568,25 @@ { "title": "Umsatzsteuer 19%", "is_default": 1, - "accounts": [ + "taxes": [ { - "type": "On Net Total", - "account_name": "Umsatzsteuer 19%", - "account_number": "1776", - "rate": 19.00 + "account_head": { + "account_name": "Umsatzsteuer 19%", + "account_number": "1776", + "tax_rate": 19.00 + } } ] }, { "title": "Umsatzsteuer 7%", - "accounts": [ + "taxes": [ { - "type": "On Net Total", - "account_name": "Umsatzsteuer 7%", - "account_number": "1771", - "tax_rate": 7.00 + "account_head": { + "account_name": "Umsatzsteuer 7%", + "account_number": "1771", + "tax_rate": 7.00 + } } ] } @@ -583,23 +595,27 @@ { "title": "Abziehbare Vorsteuer 19%", "is_default": 1, - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer 19%", - "account_number": "1576", - "root_type": "Asset", - "tax_rate": 19.00 + "account_head": { + "account_name": "Abziehbare Vorsteuer 19%", + "account_number": "1576", + "root_type": "Asset", + "tax_rate": 19.00 + } } ] }, { "title": "Abziehbare Vorsteuer 7%", - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer 7%", - "account_number": "1571", - "root_type": "Asset", - "tax_rate": 7.00 + "account_head": { + "account_name": "Abziehbare Vorsteuer 7%", + "account_number": "1571", + "root_type": "Asset", + "tax_rate": 7.00 + } } ] } @@ -610,23 +626,25 @@ { "title": "Umsatzsteuer 19%", "is_default": 1, - "accounts": [ + "taxes": [ { - "type": "On Net Total", - "account_name": "Umsatzsteuer 19%", - "account_number": "2301", - "rate": 19.00 + "account_head": { + "account_name": "Umsatzsteuer 19%", + "account_number": "2301", + "tax_rate": 19.00 + } } ] }, { "title": "Umsatzsteuer 7%", - "accounts": [ + "taxes": [ { - "type": "On Net Total", - "account_name": "Umsatzsteuer 7%", - "account_number": "2302", - "tax_rate": 7.00 + "account_head": { + "account_name": "Umsatzsteuer 7%", + "account_number": "2302", + "tax_rate": 7.00 + } } ] } @@ -635,23 +653,27 @@ { "title": "Abziehbare Vorsteuer 19%", "is_default": 1, - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer 19%", - "account_number": "1501", - "root_type": "Asset", - "tax_rate": 19.00 + "account_head": { + "account_name": "Abziehbare Vorsteuer 19%", + "account_number": "1501", + "root_type": "Asset", + "tax_rate": 19.00 + } } ] }, { "title": "Abziehbare Vorsteuer 7%", - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer 7%", - "account_number": "1502", - "root_type": "Asset", - "tax_rate": 7.00 + "account_head": { + "account_name": "Abziehbare Vorsteuer 7%", + "account_number": "1502", + "root_type": "Asset", + "tax_rate": 7.00 + } } ] } @@ -662,21 +684,23 @@ { "title": "Umsatzsteuer 19%", "is_default": 1, - "accounts": [ + "taxes": [ { - "type": "On Net Total", - "account_name": "Umsatzsteuer 19%", - "rate": 19.00 + "account_head": { + "account_name": "Umsatzsteuer 19%", + "tax_rate": 19.00 + } } ] }, { "title": "Umsatzsteuer 7%", - "accounts": [ + "taxes": [ { - "type": "On Net Total", - "account_name": "Umsatzsteuer 7%", - "tax_rate": 7.00 + "account_head": { + "account_name": "Umsatzsteuer 7%", + "tax_rate": 7.00 + } } ] } @@ -685,21 +709,25 @@ { "title": "Abziehbare Vorsteuer 19%", "is_default": 1, - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer 19%", - "root_type": "Asset", - "tax_rate": 19.00 + "account_head": { + "account_name": "Abziehbare Vorsteuer 19%", + "tax_rate": 19.00, + "root_type": "Asset" + } } ] }, { "title": "Abziehbare Vorsteuer 7%", - "accounts": [ + "taxes": [ { - "account_name": "Abziehbare Vorsteuer 7%", - "root_type": "Asset", - "tax_rate": 7.00 + "account_head": { + "account_name": "Abziehbare Vorsteuer 7%", + "root_type": "Asset", + "tax_rate": 7.00 + } } ] } @@ -798,54 +826,130 @@ "India": { "chart_of_accounts": { "*": { - "*": [ + "item_tax_templates": [ { "title": "In State GST", "is_default": 1, - "accounts": [ + "taxes": [ { - "account_name": "SGST", - "tax_rate": 9.00 + "tax_type": { + "account_name": "SGST", + "tax_rate": 9.00 + } }, { - "account_name": "CGST", - "tax_rate": 9.00 + "tax_type": { + "account_name": "CGST", + "tax_rate": 9.00 + } } ] }, { "title": "Out of State GST", - "accounts": [ + "taxes": [ { - "account_name": "IGST", - "tax_rate": 18.00 + "tax_type": { + "account_name": "IGST", + "tax_rate": 18.00 + } } ] }, { "title": "VAT 5%", - "accounts": [ + "taxes": [ { - "account_name": "VAT 5%", - "tax_rate": 5.00 + "tax_type": { + "account_name": "VAT 5%", + "tax_rate": 5.00 + } } ] }, { "title": "VAT 4%", - "accounts": [ + "taxes": [ { - "account_name": "VAT 4%", - "tax_rate": 4.00 + "tax_type": { + "account_name": "VAT 4%", + "tax_rate": 4.00 + } } ] }, { "title": "VAT 14%", - "accounts": [ + "taxes": [ { - "account_name": "VAT 14%", - "tax_rate": 14.00 + "tax_type": { + "account_name": "VAT 14%", + "tax_rate": 14.00 + } + } + ] + } + ], + "*": [ + { + "title": "In State GST", + "is_default": 1, + "taxes": [ + { + "account_head": { + "account_name": "SGST", + "tax_rate": 9.00 + } + }, + { + "account_head": { + "account_name": "CGST", + "tax_rate": 9.00 + } + } + ] + }, + { + "title": "Out of State GST", + "taxes": [ + { + "account_head": { + "account_name": "IGST", + "tax_rate": 18.00 + } + } + ] + }, + { + "title": "VAT 5%", + "taxes": [ + { + "account_head": { + "account_name": "VAT 5%", + "tax_rate": 5.00 + } + } + ] + }, + { + "title": "VAT 4%", + "taxes": [ + { + "account_head": { + "account_name": "VAT 4%", + "tax_rate": 4.00 + } + } + ] + }, + { + "title": "VAT 14%", + "taxes": [ + { + "account_head": { + "account_name": "VAT 14%", + "tax_rate": 14.00 + } } ] } diff --git a/erpnext/setup/setup_wizard/operations/taxes_setup.py b/erpnext/setup/setup_wizard/operations/taxes_setup.py index 81506c4352..429a558c58 100644 --- a/erpnext/setup/setup_wizard/operations/taxes_setup.py +++ b/erpnext/setup/setup_wizard/operations/taxes_setup.py @@ -17,40 +17,62 @@ def setup_taxes_and_charges(company_name: str, country: str): country_wise_tax = tax_data.get(country) - if country_wise_tax: - if 'chart_of_accounts' in country_wise_tax: - from_detailed_data(company_name, country_wise_tax.get('chart_of_accounts')) - else: - from_simple_data(company_name, country_wise_tax) + if not country_wise_tax: + return + + if 'chart_of_accounts' not in country_wise_tax: + country_wise_tax = simple_to_detailed(country_wise_tax) + + from_detailed_data(company_name, country_wise_tax.get('chart_of_accounts')) -def from_detailed_data(company_name, data): +def simple_to_detailed(templates): """ - Create Taxes and Charges Templates from detailed data like this: + Convert a simple taxes object into a more detailed data structure. + + Example input: { - "chart_of_accounts": { - coa_name: { - "sales_tax_templates": [ - { - 'title': '', - 'is_default': 1, - 'accounts': [ - { - 'account_name': '', - 'account_number': '', - 'root_type': '', - } - ] - } - ], - "purchase_tax_templates": [ ... ], - "item_tax_templates": [ ... ], - "*": [ ... ] - } + "France VAT 20%": { + "account_name": "VAT 20%", + "tax_rate": 20, + "default": 1 + }, + "France VAT 10%": { + "account_name": "VAT 10%", + "tax_rate": 10 } } """ + return { + 'chart_of_accounts': { + '*': { + 'item_tax_templates': [{ + 'title': title, + 'taxes': [{ + 'tax_type': { + 'account_name': data.get('account_name'), + 'tax_rate': data.get('tax_rate') + } + }] + } for title, data in templates.items()], + '*': [{ + 'title': title, + 'is_default': data.get('default', 0), + 'taxes': [{ + 'account_head': { + 'account_name': data.get('account_name'), + 'tax_rate': data.get('tax_rate') + } + }] + } for title, data in templates.items()] + } + } + } + + +def from_detailed_data(company_name, data): + """Create Taxes and Charges Templates from detailed data.""" coa_name = frappe.db.get_value('Company', company_name, 'chart_of_accounts') tax_templates = data.get(coa_name) or data.get('*') sales_tax_templates = tax_templates.get('sales_tax_templates') or tax_templates.get('*') @@ -59,85 +81,44 @@ def from_detailed_data(company_name, data): if sales_tax_templates: for template in sales_tax_templates: - make_tax_template(company_name, 'Sales Taxes and Charges Template', template) + make_taxes_and_charges_template(company_name, 'Sales Taxes and Charges Template', template) if purchase_tax_templates: for template in purchase_tax_templates: - make_tax_template(company_name, 'Purchase Taxes and Charges Template', template) + make_taxes_and_charges_template(company_name, 'Purchase Taxes and Charges Template', template) if item_tax_templates: for template in item_tax_templates: make_item_tax_template(company_name, template) -def from_simple_data(company_name, data): - """ - Create Taxes and Charges Templates from simple data like this: +def make_taxes_and_charges_template(company_name, doctype, template): + template['company'] = company_name + template['doctype'] = doctype - "Austria Tax": { - "account_name": "VAT", - "tax_rate": 20.00 - } - """ - for template_name, tax_data in data.items(): - template = { - 'title': template_name, - 'is_default': tax_data.get('default'), - 'accounts': [ - { - 'account_name': tax_data.get('account_name'), - 'tax_rate': tax_data.get('tax_rate') - } - ] - } - make_tax_template(company_name, 'Sales Taxes and Charges Template', template) - make_tax_template(company_name, 'Purchase Taxes and Charges Template', template) - make_item_tax_template(company_name, template) - - -def make_tax_template(company_name, doctype, template): if frappe.db.exists(doctype, {'title': template.get('title'), 'company': company_name}): return - accounts = get_or_create_accounts(company_name, template.get('accounts')) - - # Get all fields of the Taxes and Charges Template - tax_template = {'doctype': doctype} - tax_template_fields = frappe.get_meta(doctype).fields - tax_template_fieldnames = [field.fieldname for field in tax_template_fields] - - # Get all fields of the taxes child table - table_doctype = [field.options for field in tax_template_fields if field.fieldname=='taxes'][0] - table_fields = frappe.get_meta(table_doctype).fields - table_field_names = [field.fieldname for field in table_fields] - - # Check if field exists as a key in the import data and, if yes, set the - # value accordingly - for field in tax_template_fieldnames: - if field in template: - tax_template[field] = template.get(field) - - # However, company always fixed and taxes table must be empty to start with - tax_template['company'] = company_name - tax_template['taxes'] = [] - - for account in accounts: - row = { + for tax_row in template.get('taxes'): + account_data = tax_row.get('account_head') + tax_row_defaults = { 'category': 'Total', - 'charge_type': 'On Net Total', - 'account_head': account.get('name'), - 'description': '{0} @ {1}'.format(account.get('account_name'), account.get('tax_rate')), - 'rate': account.get('tax_rate') + 'charge_type': 'On Net Total' } - # Check if field exists as a key in the import data and, if yes, set the - # value accordingly - for field in table_field_names: - if field in account: - row[field] = account.get(field) - tax_template['taxes'].append(row) + # if account_head is a dict, search or create the account and get it's name + if isinstance(account_data, dict): + tax_row_defaults['description'] = '{0} @ {1}'.format(account_data.get('account_name'), account_data.get('tax_rate')) + tax_row_defaults['rate'] = account_data.get('tax_rate') + account = get_or_create_account(company_name, account_data) + tax_row['account_head'] = account.name - return frappe.get_doc(tax_template).insert(ignore_permissions=True) + # use the default value if nothing other is specified + for fieldname, default_value in tax_row_defaults.items(): + if fieldname not in tax_row: + tax_row[fieldname] = default_value + + return frappe.get_doc(template).insert(ignore_permissions=True) def make_item_tax_template(company_name, template): @@ -147,111 +128,105 @@ def make_item_tax_template(company_name, template): differently from Sales and Purchase Tax Templates. """ doctype = 'Item Tax Template' + template['company'] = company_name + template['doctype'] = doctype + if frappe.db.exists(doctype, {'title': template.get('title'), 'company': company_name}): return - accounts = get_or_create_accounts(company_name, template.get('accounts')) + for tax_row in template.get('taxes'): + account_data = tax_row.get('tax_type') - item_tax_template = { - 'doctype': doctype, - 'title': template.get('title'), - 'company': company_name, - 'taxes': [{ - 'tax_type': account.get('name'), - 'tax_rate': account.get('tax_rate') - } for account in accounts] - } + # if tax_type is a dict, search or create the account and get it's name + if isinstance(account_data, dict): + account = get_or_create_account(company_name, account_data) + tax_row['tax_type'] = account.name + if 'tax_rate' not in tax_row: + tax_row['tax_rate'] = account_data.get('tax_rate') - return frappe.get_doc(item_tax_template).insert(ignore_permissions=True) + return frappe.get_doc(template).insert(ignore_permissions=True) -def get_or_create_accounts(company: str, account_data: list): - for account in account_data: - if 'creation' in account: - # Hack to check if account already contains a real Account doc - # or just the attibutes from country_wise_tax.json - continue - - # tax_rate should survive the following lines because it might not be - # specified in an existing account or different rates might get booked - # onto the same account. - tax_rate = account.get('tax_rate') - doc = get_or_create_account(company, account) - account.update(doc.as_dict()) - account['tax_rate'] = tax_rate - - return account_data - - -def get_or_create_account(company, account_data): +def get_or_create_account(company_name, account): """ Check if account already exists. If not, create it. Return a tax account or None. """ - root_type = account_data.get('root_type', 'Liability') - account_name = account_data.get('account_name') - account_number = account_data.get('account_number') + default_root_type = 'Liability' + root_type = account.get('root_type', default_root_type) existing_accounts = frappe.get_list('Account', filters={ - 'company': company, + 'company': company_name, 'root_type': root_type }, or_filters={ - 'account_name': account_name, - 'account_number': account_number + 'account_name': account.get('account_name'), + 'account_number': account.get('account_number') } ) if existing_accounts: return frappe.get_doc('Account', existing_accounts[0].name) - tax_group = get_or_create_tax_account_group(company, root_type) - full_account_data = { - 'doctype': 'Account', - 'account_name': account_name, - 'account_number': account_number, - 'tax_rate': account_data.get('tax_rate'), - 'company': company, - 'parent_account': tax_group, - 'is_group': 0, - 'report_type': 'Balance Sheet', - 'root_type': root_type, - 'account_type': 'Tax' - } - return frappe.get_doc(full_account_data).insert(ignore_permissions=True, ignore_mandatory=True) + tax_group = get_or_create_tax_group(company_name, root_type) + + account['doctype'] = 'Account' + account['company'] = company_name + account['parent_account'] = tax_group + account['report_type'] = 'Balance Sheet' + account['account_type'] = 'Tax' + account['root_type'] = root_type + account['is_group'] = 0 + + return frappe.get_doc(account).insert(ignore_permissions=True, ignore_mandatory=True) -def get_or_create_tax_account_group(company, root_type): - tax_group = frappe.db.get_value('Account', { +def get_or_create_tax_group(company_name, root_type): + # Look for a group account of type 'Tax' + tax_group_name = frappe.db.get_value('Account', { 'is_group': 1, 'root_type': root_type, 'account_type': 'Tax', - 'company': company + 'company': company_name }) - if tax_group: - return tax_group + if tax_group_name: + return tax_group_name - root = frappe.get_list('Account', { + # Look for a group account named 'Duties and Taxes' or 'Tax Assets' + account_name = _('Duties and Taxes') if root_type == 'Liability' else _('Tax Assets') + tax_group_name = frappe.db.get_value('Account', { 'is_group': 1, 'root_type': root_type, - 'company': company, + 'account_name': account_name, + 'company': company_name + }) + + if tax_group_name: + return tax_group_name + + # Create a new group account named 'Duties and Taxes' or 'Tax Assets' just + # below the root account + root_account = frappe.get_list('Account', { + 'is_group': 1, + 'root_type': root_type, + 'company': company_name, 'report_type': 'Balance Sheet', 'parent_account': ('is', 'not set') - }, limit=1)[0].name + }, limit=1)[0] - doc = frappe.get_doc({ + tax_group_account = frappe.get_doc({ 'doctype': 'Account', - 'company': company, + 'company': company_name, 'is_group': 1, 'report_type': 'Balance Sheet', 'root_type': root_type, 'account_type': 'Tax', - 'account_name': _('Duties and Taxes') if root_type == 'Liability' else _('Tax Assets'), - 'parent_account': root + 'account_name': account_name, + 'parent_account': root_account.name }).insert(ignore_permissions=True) - tax_group = doc.name + tax_group_name = tax_group_account.name - return tax_group + return tax_group_name From 39b1cd827a5e3ba04c29189426e5f85f7fe77daf Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 15 Apr 2021 18:54:29 +0530 Subject: [PATCH 04/31] fix: Additional Salary component amount not getting set (#25355) --- erpnext/payroll/doctype/salary_slip/salary_slip.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index 539f2c56d3..afdf081ac8 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -633,6 +633,8 @@ class SalarySlip(TransactionBase): if additional_salary: component_row.default_amount = 0 + component_row.additional_amount = amount + component_row.additional_salary = additional_salary.name component_row.deduct_full_tax_on_selected_payroll_date = \ additional_salary.deduct_full_tax_on_selected_payroll_date else: From 597bb8be184778748980ef453c7c48119317ff2f Mon Sep 17 00:00:00 2001 From: 18alantom <2.alan.tom@gmail.com> Date: Thu, 15 Apr 2021 20:32:45 +0530 Subject: [PATCH 05/31] fix: remove pickup_to, pickup_from and get_pickup_time relies on server-side validation instead js controller --- erpnext/stock/doctype/shipment/shipment.js | 37 ---------------------- 1 file changed, 37 deletions(-) diff --git a/erpnext/stock/doctype/shipment/shipment.js b/erpnext/stock/doctype/shipment/shipment.js index 7af16af898..ce2906ecbe 100644 --- a/erpnext/stock/doctype/shipment/shipment.js +++ b/erpnext/stock/doctype/shipment/shipment.js @@ -363,43 +363,6 @@ frappe.ui.form.on('Shipment', { if (frm.doc.pickup_date < frappe.datetime.get_today()) { frappe.throw(__("Pickup Date cannot be before this day")); } - if (frm.doc.pickup_date == frappe.datetime.get_today()) { - var pickup_time = frm.events.get_pickup_time(frm); - frm.set_value("pickup_from", pickup_time); - frm.trigger('set_pickup_to_time'); - } - }, - pickup_from: function(frm) { - var pickup_time = frm.events.get_pickup_time(frm); - if (frm.doc.pickup_from && frm.doc.pickup_date == frappe.datetime.get_today()) { - let current_hour = pickup_time.split(':')[0]; - let current_min = pickup_time.split(':')[1]; - let pickup_hour = frm.doc.pickup_from.split(':')[0]; - let pickup_min = frm.doc.pickup_from.split(':')[1]; - if (pickup_hour < current_hour || (pickup_hour == current_hour && pickup_min < current_min)) { - frm.set_value("pickup_from", pickup_time); - frappe.throw(__("Pickup Time cannot be in the past")); - } - } - frm.trigger('set_pickup_to_time'); - }, - get_pickup_time: function() { - let current_hour = new Date().getHours(); - let current_min = new Date().toLocaleString('en-US', {minute: 'numeric'}); - if (current_min < 30) { - current_min = '30'; - } else { - current_min = '00'; - current_hour = Number(current_hour)+1; - } - let pickup_time = current_hour +':'+ current_min; - return pickup_time; - }, - set_pickup_to_time: function(frm) { - let pickup_to_hour = Number(frm.doc.pickup_from.split(':')[0])+5; - let pickup_to_min = frm.doc.pickup_from.split(':')[1]; - let pickup_to = pickup_to_hour +':'+ pickup_to_min; - frm.set_value("pickup_to", pickup_to); }, clear_pickup_fields: function(frm) { let fields = ["pickup_address_name", "pickup_contact_name", "pickup_address", "pickup_contact", "pickup_contact_email", "pickup_contact_person"]; From 6179cc1311df15ea0459ec7c9a8e65e2b2d3086d Mon Sep 17 00:00:00 2001 From: 18alantom <2.alan.tom@gmail.com> Date: Thu, 15 Apr 2021 20:36:28 +0530 Subject: [PATCH 06/31] fix: make pickup_to and pickup_from mandatory fields --- erpnext/stock/doctype/shipment/shipment.json | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/erpnext/stock/doctype/shipment/shipment.json b/erpnext/stock/doctype/shipment/shipment.json index 76c331c5c2..a33cbc288c 100644 --- a/erpnext/stock/doctype/shipment/shipment.json +++ b/erpnext/stock/doctype/shipment/shipment.json @@ -275,14 +275,16 @@ "default": "09:00", "fieldname": "pickup_from", "fieldtype": "Time", - "label": "Pickup from" + "label": "Pickup from", + "reqd": 1 }, { "allow_on_submit": 1, "default": "17:00", "fieldname": "pickup_to", "fieldtype": "Time", - "label": "Pickup to" + "label": "Pickup to", + "reqd": 1 }, { "fieldname": "column_break_36", @@ -431,7 +433,7 @@ ], "is_submittable": 1, "links": [], - "modified": "2020-12-25 15:02:34.891976", + "modified": "2021-04-13 17:14:18.181818", "modified_by": "Administrator", "module": "Stock", "name": "Shipment", @@ -469,4 +471,4 @@ "sort_field": "modified", "sort_order": "DESC", "track_changes": 1 -} \ No newline at end of file +} From c0db286dc1572d591a9a6c7c70620ffebfbd28d2 Mon Sep 17 00:00:00 2001 From: Afshan <33727827+AfshanKhan@users.noreply.github.com> Date: Thu, 15 Apr 2021 23:48:25 +0530 Subject: [PATCH 07/31] fix: filter for employees in salary slip (#25360) --- erpnext/payroll/doctype/salary_slip/salary_slip.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.js b/erpnext/payroll/doctype/salary_slip/salary_slip.js index a0ddd39ca2..5258f3aff9 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.js +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.js @@ -40,7 +40,9 @@ frappe.ui.form.on("Salary Slip", { frm.set_query("employee", function() { return { query: "erpnext.controllers.queries.employee_query", - filters: frm.doc.company + filters: { + company: frm.doc.company + } }; }); }, From 9d9c256e70ebf00c493f5131179700c3a17e4404 Mon Sep 17 00:00:00 2001 From: Anupam Date: Fri, 16 Apr 2021 00:11:40 +0530 Subject: [PATCH 08/31] feat: added Disable Rounded Total in sales transactions --- .../doctype/sales_invoice/sales_invoice.json | 14 +++++++++++++- .../selling/doctype/sales_order/sales_order.json | 14 +++++++++++++- .../stock/doctype/delivery_note/delivery_note.json | 14 +++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json index d382386a32..c6c67b4ddc 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json @@ -118,6 +118,7 @@ "in_words", "total_advance", "outstanding_amount", + "disable_rounded_total", "advances_section", "allocate_advances_automatically", "get_advances", @@ -1109,6 +1110,7 @@ "reqd": 1 }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "base_rounding_adjustment", "fieldtype": "Currency", "hide_days": 1, @@ -1120,6 +1122,7 @@ "read_only": 1 }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "base_rounded_total", "fieldtype": "Currency", "hide_days": 1, @@ -1168,6 +1171,7 @@ "reqd": 1 }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "rounding_adjustment", "fieldtype": "Currency", "hide_days": 1, @@ -1180,6 +1184,7 @@ }, { "bold": 1, + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "rounded_total", "fieldtype": "Currency", "hide_days": 1, @@ -1945,6 +1950,13 @@ "fieldtype": "Link", "label": "Set Target Warehouse", "options": "Warehouse" + }, + { + "default": "0", + "depends_on": "grand_total", + "fieldname": "disable_rounded_total", + "fieldtype": "Check", + "label": "Disable Rounded Total" } ], "icon": "fa fa-file-text", @@ -1957,7 +1969,7 @@ "link_fieldname": "consolidated_invoice" } ], - "modified": "2021-03-31 15:42:26.261540", + "modified": "2021-04-15 23:57:58.766651", "modified_by": "Administrator", "module": "Accounts", "name": "Sales Invoice", diff --git a/erpnext/selling/doctype/sales_order/sales_order.json b/erpnext/selling/doctype/sales_order/sales_order.json index 0a5c6651ba..762b6f1d6c 100644 --- a/erpnext/selling/doctype/sales_order/sales_order.json +++ b/erpnext/selling/doctype/sales_order/sales_order.json @@ -98,6 +98,7 @@ "rounded_total", "in_words", "advance_paid", + "disable_rounded_total", "packing_list", "packed_items", "payment_schedule_section", @@ -901,6 +902,7 @@ "width": "150px" }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "base_rounding_adjustment", "fieldtype": "Currency", "hide_days": 1, @@ -912,6 +914,7 @@ "read_only": 1 }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "base_rounded_total", "fieldtype": "Currency", "hide_days": 1, @@ -961,6 +964,7 @@ "width": "150px" }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "rounding_adjustment", "fieldtype": "Currency", "hide_days": 1, @@ -973,6 +977,7 @@ }, { "bold": 1, + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "rounded_total", "fieldtype": "Currency", "hide_days": 1, @@ -1474,13 +1479,20 @@ "label": "Represents Company", "options": "Company", "read_only": 1 + }, + { + "default": "0", + "depends_on": "grand_total", + "fieldname": "disable_rounded_total", + "fieldtype": "Check", + "label": "Disable Rounded Total" } ], "icon": "fa fa-file-text", "idx": 105, "is_submittable": 1, "links": [], - "modified": "2021-01-20 23:40:39.929296", + "modified": "2021-04-15 23:55:13.439068", "modified_by": "Administrator", "module": "Selling", "name": "Sales Order", diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.json b/erpnext/stock/doctype/delivery_note/delivery_note.json index f595aade91..280fde158f 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.json +++ b/erpnext/stock/doctype/delivery_note/delivery_note.json @@ -99,6 +99,7 @@ "rounding_adjustment", "rounded_total", "in_words", + "disable_rounded_total", "terms_section_break", "tc_name", "terms", @@ -768,6 +769,7 @@ "width": "150px" }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "base_rounding_adjustment", "fieldtype": "Currency", "label": "Rounding Adjustment (Company Currency)", @@ -777,6 +779,7 @@ "read_only": 1 }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "base_rounded_total", "fieldtype": "Currency", "label": "Rounded Total (Company Currency)", @@ -819,6 +822,7 @@ "width": "150px" }, { + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "rounding_adjustment", "fieldtype": "Currency", "label": "Rounding Adjustment", @@ -829,6 +833,7 @@ }, { "bold": 1, + "depends_on": "eval:!doc.disable_rounded_total", "fieldname": "rounded_total", "fieldtype": "Currency", "label": "Rounded Total", @@ -1271,13 +1276,20 @@ "label": "Represents Company", "options": "Company", "read_only": 1 + }, + { + "default": "0", + "depends_on": "grand_total", + "fieldname": "disable_rounded_total", + "fieldtype": "Check", + "label": "Disable Rounded Total" } ], "icon": "fa fa-truck", "idx": 146, "is_submittable": 1, "links": [], - "modified": "2020-12-26 17:07:59.194403", + "modified": "2021-04-15 23:55:49.620641", "modified_by": "Administrator", "module": "Stock", "name": "Delivery Note", From b1aad63a9910367fecab17efb1193453db717a88 Mon Sep 17 00:00:00 2001 From: Jannat Patel <31363128+pateljannat@users.noreply.github.com> Date: Fri, 16 Apr 2021 16:08:22 +0530 Subject: [PATCH 09/31] fix: leave policy in leave allocation (#25334) Co-authored-by: Rucha Mahabal --- erpnext/hr/doctype/leave_allocation/leave_allocation.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/erpnext/hr/doctype/leave_allocation/leave_allocation.json b/erpnext/hr/doctype/leave_allocation/leave_allocation.json index 3a300c0d63..ae02c512c2 100644 --- a/erpnext/hr/doctype/leave_allocation/leave_allocation.json +++ b/erpnext/hr/doctype/leave_allocation/leave_allocation.json @@ -218,8 +218,7 @@ "fieldname": "leave_policy_assignment", "fieldtype": "Link", "label": "Leave Policy Assignment", - "options": "Leave Policy Assignment", - "read_only": 1 + "options": "Leave Policy Assignment" }, { "fetch_from": "employee.company", @@ -236,7 +235,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2021-01-04 18:46:13.184104", + "modified": "2021-04-14 15:28:26.335104", "modified_by": "Administrator", "module": "HR", "name": "Leave Allocation", From ede339f80bf514c8f7e5372be3e91567d063b26a Mon Sep 17 00:00:00 2001 From: Marica Date: Fri, 16 Apr 2021 18:42:54 +0530 Subject: [PATCH 10/31] fix: Serial No not updated correctly via Inter Company Stock Transfer (#25006) * fix: Serial No not updated correctly via Inter Company Stock Transfer * chore: Added More Test Cases for inter company Serial Transfer * fix: Test for serial no duplication - fixed serial no test - made errors more meaningful on serial no validation * fix: Stock Reco Test Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- .../purchase_receipt/test_purchase_receipt.py | 1 + erpnext/stock/doctype/serial_no/serial_no.py | 33 ++++- .../stock/doctype/serial_no/test_serial_no.py | 129 +++++++++++++++++- .../stock_reconciliation.py | 2 +- .../test_stock_reconciliation.py | 4 +- 5 files changed, 159 insertions(+), 10 deletions(-) diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index 7f0c3fa801..16eea24f84 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -582,6 +582,7 @@ class TestPurchaseReceipt(unittest.TestCase): serial_no=serial_no, basic_rate=100, do_not_submit=True) se.submit() + se.cancel() dn.cancel() pr1.cancel() diff --git a/erpnext/stock/doctype/serial_no/serial_no.py b/erpnext/stock/doctype/serial_no/serial_no.py index c8d8ca9e17..c02dd2e518 100644 --- a/erpnext/stock/doctype/serial_no/serial_no.py +++ b/erpnext/stock/doctype/serial_no/serial_no.py @@ -14,6 +14,7 @@ from frappe import _, ValidationError from erpnext.controllers.stock_controller import StockController from six import string_types from six.moves import map + class SerialNoCannotCreateDirectError(ValidationError): pass class SerialNoCannotCannotChangeError(ValidationError): pass class SerialNoNotRequiredError(ValidationError): pass @@ -322,11 +323,35 @@ def validate_serial_no(sle, item_det): frappe.throw(_("Serial Nos Required for Serialized Item {0}").format(sle.item_code), SerialNoRequiredError) elif serial_nos: + # SLE is being cancelled and has serial nos for serial_no in serial_nos: - sr = frappe.db.get_value("Serial No", serial_no, ["name", "warehouse"], as_dict=1) - if sr and cint(sle.actual_qty) < 0 and sr.warehouse != sle.warehouse: - frappe.throw(_("Cannot cancel {0} {1} because Serial No {2} does not belong to the warehouse {3}") - .format(sle.voucher_type, sle.voucher_no, serial_no, sle.warehouse)) + check_serial_no_validity_on_cancel(serial_no, sle) + +def check_serial_no_validity_on_cancel(serial_no, sle): + sr = frappe.db.get_value("Serial No", serial_no, ["name", "warehouse", "company", "status"], as_dict=1) + sr_link = frappe.utils.get_link_to_form("Serial No", serial_no) + doc_link = frappe.utils.get_link_to_form(sle.voucher_type, sle.voucher_no) + actual_qty = cint(sle.actual_qty) + is_stock_reco = sle.voucher_type == "Stock Reconciliation" + msg = None + + if sr and (actual_qty < 0 or is_stock_reco) and sr.warehouse != sle.warehouse: + # receipt(inward) is being cancelled + msg = _("Cannot cancel {0} {1} as Serial No {2} does not belong to the warehouse {3}").format( + sle.voucher_type, doc_link, sr_link, frappe.bold(sle.warehouse)) + elif sr and actual_qty > 0 and not is_stock_reco: + # delivery is being cancelled, check for warehouse. + if sr.warehouse: + # serial no is active in another warehouse/company. + msg = _("Cannot cancel {0} {1} as Serial No {2} is active in warehouse {3}").format( + sle.voucher_type, doc_link, sr_link, frappe.bold(sr.warehouse)) + elif sr.company != sle.company and sr.status == "Delivered": + # serial no is inactive (allowed) or delivered from another company (block). + msg = _("Cannot cancel {0} {1} as Serial No {2} does not belong to the company {3}").format( + sle.voucher_type, doc_link, sr_link, frappe.bold(sle.company)) + + if msg: + frappe.throw(msg, title=_("Cannot cancel")) def validate_material_transfer_entry(sle_doc): sle_doc.update({ diff --git a/erpnext/stock/doctype/serial_no/test_serial_no.py b/erpnext/stock/doctype/serial_no/test_serial_no.py index ed70790b2c..cde7fe07c6 100644 --- a/erpnext/stock/doctype/serial_no/test_serial_no.py +++ b/erpnext/stock/doctype/serial_no/test_serial_no.py @@ -40,16 +40,139 @@ class TestSerialNo(unittest.TestCase): se = make_serialized_item(target_warehouse="_Test Warehouse - _TC") serial_nos = get_serial_nos(se.get("items")[0].serial_no) - create_delivery_note(item_code="_Test Serialized Item With Series", qty=1, serial_no=serial_nos[0]) + dn = create_delivery_note(item_code="_Test Serialized Item With Series", qty=1, serial_no=serial_nos[0]) + + serial_no = frappe.get_doc("Serial No", serial_nos[0]) + + # check Serial No details after delivery + self.assertEqual(serial_no.status, "Delivered") + self.assertEqual(serial_no.warehouse, None) + self.assertEqual(serial_no.company, "_Test Company") + self.assertEqual(serial_no.delivery_document_type, "Delivery Note") + self.assertEqual(serial_no.delivery_document_no, dn.name) wh = create_warehouse("_Test Warehouse", company="_Test Company 1") - make_purchase_receipt(item_code="_Test Serialized Item With Series", qty=1, serial_no=serial_nos[0], + pr = make_purchase_receipt(item_code="_Test Serialized Item With Series", qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh) - serial_no = frappe.db.get_value("Serial No", serial_nos[0], ["warehouse", "company"], as_dict=1) + serial_no.reload() + # check Serial No details after purchase in second company + self.assertEqual(serial_no.status, "Active") self.assertEqual(serial_no.warehouse, wh) self.assertEqual(serial_no.company, "_Test Company 1") + self.assertEqual(serial_no.purchase_document_type, "Purchase Receipt") + self.assertEqual(serial_no.purchase_document_no, pr.name) + + def test_inter_company_transfer_intermediate_cancellation(self): + """ + Receive into and Deliver Serial No from one company. + Then Receive into and Deliver from second company. + Try to cancel intermediate receipts/deliveries to test if it is blocked. + """ + se = make_serialized_item(target_warehouse="_Test Warehouse - _TC") + serial_nos = get_serial_nos(se.get("items")[0].serial_no) + + sn_doc = frappe.get_doc("Serial No", serial_nos[0]) + + # check Serial No details after purchase in first company + self.assertEqual(sn_doc.status, "Active") + self.assertEqual(sn_doc.company, "_Test Company") + self.assertEqual(sn_doc.warehouse, "_Test Warehouse - _TC") + self.assertEqual(sn_doc.purchase_document_no, se.name) + + dn = create_delivery_note(item_code="_Test Serialized Item With Series", + qty=1, serial_no=serial_nos[0]) + sn_doc.reload() + # check Serial No details after delivery from **first** company + self.assertEqual(sn_doc.status, "Delivered") + self.assertEqual(sn_doc.company, "_Test Company") + self.assertEqual(sn_doc.warehouse, None) + self.assertEqual(sn_doc.delivery_document_no, dn.name) + + # try cancelling the first Serial No Receipt, even though it is delivered + # block cancellation is Serial No is out of the warehouse + self.assertRaises(frappe.ValidationError, se.cancel) + + # receive serial no in second company + wh = create_warehouse("_Test Warehouse", company="_Test Company 1") + pr = make_purchase_receipt(item_code="_Test Serialized Item With Series", + qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh) + sn_doc.reload() + + self.assertEqual(sn_doc.warehouse, wh) + # try cancelling the delivery from the first company + # block cancellation as Serial No belongs to different company + self.assertRaises(frappe.ValidationError, dn.cancel) + + # deliver from second company + dn_2 = create_delivery_note(item_code="_Test Serialized Item With Series", + qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh) + sn_doc.reload() + + # check Serial No details after delivery from **second** company + self.assertEqual(sn_doc.status, "Delivered") + self.assertEqual(sn_doc.company, "_Test Company 1") + self.assertEqual(sn_doc.warehouse, None) + self.assertEqual(sn_doc.delivery_document_no, dn_2.name) + + # cannot cancel any intermediate document before last Delivery Note + self.assertRaises(frappe.ValidationError, se.cancel) + self.assertRaises(frappe.ValidationError, dn.cancel) + self.assertRaises(frappe.ValidationError, pr.cancel) + + def test_inter_company_transfer_fallback_on_cancel(self): + """ + Test Serial No state changes on cancellation. + If Delivery cancelled, it should fall back on last Receipt in the same company. + If Receipt is cancelled, it should be Inactive in the same company. + """ + # Receipt in **first** company + se = make_serialized_item(target_warehouse="_Test Warehouse - _TC") + serial_nos = get_serial_nos(se.get("items")[0].serial_no) + sn_doc = frappe.get_doc("Serial No", serial_nos[0]) + + # Delivery from first company + dn = create_delivery_note(item_code="_Test Serialized Item With Series", + qty=1, serial_no=serial_nos[0]) + + # Receipt in **second** company + wh = create_warehouse("_Test Warehouse", company="_Test Company 1") + pr = make_purchase_receipt(item_code="_Test Serialized Item With Series", + qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh) + + # Delivery from second company + dn_2 = create_delivery_note(item_code="_Test Serialized Item With Series", + qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh) + sn_doc.reload() + + self.assertEqual(sn_doc.status, "Delivered") + self.assertEqual(sn_doc.company, "_Test Company 1") + self.assertEqual(sn_doc.delivery_document_no, dn_2.name) + + dn_2.cancel() + sn_doc.reload() + # Fallback on Purchase Receipt if Delivery is cancelled + self.assertEqual(sn_doc.status, "Active") + self.assertEqual(sn_doc.company, "_Test Company 1") + self.assertEqual(sn_doc.warehouse, wh) + self.assertEqual(sn_doc.purchase_document_no, pr.name) + + pr.cancel() + sn_doc.reload() + # Inactive in same company if Receipt cancelled + self.assertEqual(sn_doc.status, "Inactive") + self.assertEqual(sn_doc.company, "_Test Company 1") + self.assertEqual(sn_doc.warehouse, None) + + dn.cancel() + sn_doc.reload() + # Fallback on Purchase Receipt in FIRST company if + # Delivery from FIRST company is cancelled + self.assertEqual(sn_doc.status, "Active") + self.assertEqual(sn_doc.company, "_Test Company") + self.assertEqual(sn_doc.warehouse, "_Test Warehouse - _TC") + self.assertEqual(sn_doc.purchase_document_no, se.name) def tearDown(self): frappe.db.rollback() \ No newline at end of file diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index b452e96c5e..1396f19d3f 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -398,7 +398,7 @@ class StockReconciliation(StockController): merge_similar_entries = {} for d in sl_entries: - if not d.serial_no or d.actual_qty < 0: + if not d.serial_no or flt(d.get("actual_qty")) < 0: new_sl_entries.append(d) continue diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index 6690c6a606..36380b838b 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -32,7 +32,7 @@ class TestStockReconciliation(unittest.TestCase): company = frappe.db.get_value('Warehouse', 'Stores - TCP1', 'company') # [[qty, valuation_rate, posting_date, # posting_time, expected_stock_value, bin_qty, bin_valuation]] - + input_data = [ [50, 1000, "2012-12-26", "12:00"], [25, 900, "2012-12-26", "12:00"], @@ -86,7 +86,7 @@ class TestStockReconciliation(unittest.TestCase): se1.cancel() def test_get_items(self): - create_warehouse("_Test Warehouse Group 1", + create_warehouse("_Test Warehouse Group 1", {"is_group": 1, "company": "_Test Company", "parent_warehouse": "All Warehouses - _TC"}) create_warehouse("_Test Warehouse Ledger 1", {"is_group": 0, "parent_warehouse": "_Test Warehouse Group 1 - _TC", "company": "_Test Company"}) From adf974810d03b40ad282197bd77c1513b77ef91e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 16 Apr 2021 21:15:50 +0530 Subject: [PATCH 11/31] fix: equality check instead of assignment in cart --- erpnext/shopping_cart/cart.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/shopping_cart/cart.py b/erpnext/shopping_cart/cart.py index 8515db3300..56afe95efd 100644 --- a/erpnext/shopping_cart/cart.py +++ b/erpnext/shopping_cart/cart.py @@ -230,12 +230,12 @@ def update_cart_address(address_type, address_name): if address_type.lower() == "billing": quotation.customer_address = address_name quotation.address_display = address_display - quotation.shipping_address_name == quotation.shipping_address_name or address_name + quotation.shipping_address_name = quotation.shipping_address_name or address_name address_doc = next((doc for doc in get_billing_addresses() if doc["name"] == address_name), None) elif address_type.lower() == "shipping": quotation.shipping_address_name = address_name quotation.shipping_address = address_display - quotation.customer_address == quotation.customer_address or address_name + quotation.customer_address = quotation.customer_address or address_name address_doc = next((doc for doc in get_shipping_addresses() if doc["name"] == address_name), None) apply_cart_settings(quotation=quotation) From 67e647232cf289858d1aa3dbea8fe94d5ba746d2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 16 Apr 2021 21:44:49 +0530 Subject: [PATCH 12/31] ci(semgrep): Add semgrep testing (#24871) Adds semgrep testing in CI. Refer to: - https://github.com/frappe/frappe/pull/12524 - https://github.com/frappe/frappe/pull/12577 --- .github/helper/semgrep_rules/README.md | 38 +++++++++++ .../semgrep_rules/frappe_correctness.py | 28 +++++++++ .../semgrep_rules/frappe_correctness.yml | 56 +++++++++++++++++ .github/helper/semgrep_rules/security.py | 6 ++ .github/helper/semgrep_rules/security.yml | 25 ++++++++ .github/helper/semgrep_rules/translate.js | 37 +++++++++++ .github/helper/semgrep_rules/translate.py | 53 ++++++++++++++++ .github/helper/semgrep_rules/translate.yml | 63 +++++++++++++++++++ .github/helper/semgrep_rules/ux.py | 31 +++++++++ .github/helper/semgrep_rules/ux.yml | 15 +++++ .github/workflows/semgrep.yml | 24 +++++++ 11 files changed, 376 insertions(+) create mode 100644 .github/helper/semgrep_rules/README.md create mode 100644 .github/helper/semgrep_rules/frappe_correctness.py create mode 100644 .github/helper/semgrep_rules/frappe_correctness.yml create mode 100644 .github/helper/semgrep_rules/security.py create mode 100644 .github/helper/semgrep_rules/security.yml create mode 100644 .github/helper/semgrep_rules/translate.js create mode 100644 .github/helper/semgrep_rules/translate.py create mode 100644 .github/helper/semgrep_rules/translate.yml create mode 100644 .github/helper/semgrep_rules/ux.py create mode 100644 .github/helper/semgrep_rules/ux.yml create mode 100644 .github/workflows/semgrep.yml diff --git a/.github/helper/semgrep_rules/README.md b/.github/helper/semgrep_rules/README.md new file mode 100644 index 0000000000..670d8d280f --- /dev/null +++ b/.github/helper/semgrep_rules/README.md @@ -0,0 +1,38 @@ +# Semgrep linting + +## What is semgrep? +Semgrep or "semantic grep" is language agnostic static analysis tool. In simple terms semgrep is syntax-aware `grep`, so unlike regex it doesn't get confused by different ways of writing same thing or whitespaces or code split in multiple lines etc. + +Example: + +To check if a translate function is using f-string or not the regex would be `r"_\(\s*f[\"']"` while equivalent rule in semgrep would be `_(f"...")`. As semgrep knows grammer of language it takes care of unnecessary whitespace, type of quotation marks etc. + +You can read more such examples in `.github/helper/semgrep_rules` directory. + +# Why/when to use this? +We want to maintain quality of contributions, at the same time remembering all the good practices can be pain to deal with while evaluating contributions. Using semgrep if you can translate "best practice" into a rule then it can automate the task for us. + +## Running locally + +Install semgrep using homebrew `brew install semgrep` or pip `pip install semgrep`. + +To run locally use following command: + +`semgrep --config=.github/helper/semgrep_rules [file/folder names]` + +## Testing +semgrep allows testing the tests. Refer to this page: https://semgrep.dev/docs/writing-rules/testing-rules/ + +When writing new rules you should write few positive and few negative cases as shown in the guide and current tests. + +To run current tests: `semgrep --test --test-ignore-todo .github/helper/semgrep_rules` + + +## Reference + +If you are new to Semgrep read following pages to get started on writing/modifying rules: + +- https://semgrep.dev/docs/getting-started/ +- https://semgrep.dev/docs/writing-rules/rule-syntax +- https://semgrep.dev/docs/writing-rules/pattern-examples/ +- https://semgrep.dev/docs/writing-rules/rule-ideas/#common-use-cases diff --git a/.github/helper/semgrep_rules/frappe_correctness.py b/.github/helper/semgrep_rules/frappe_correctness.py new file mode 100644 index 0000000000..4798b927f8 --- /dev/null +++ b/.github/helper/semgrep_rules/frappe_correctness.py @@ -0,0 +1,28 @@ +import frappe +from frappe import _, flt + +from frappe.model.document import Document + + +def on_submit(self): + if self.value_of_goods == 0: + frappe.throw(_('Value of goods cannot be 0')) + # ruleid: frappe-modifying-after-submit + self.status = 'Submitted' + +def on_submit(self): + if flt(self.per_billed) < 100: + self.update_billing_status() + else: + # todook: frappe-modifying-after-submit + self.status = "Completed" + self.db_set("status", "Completed") + +class TestDoc(Document): + pass + + def validate(self): + #ruleid: frappe-modifying-child-tables-while-iterating + for item in self.child_table: + if item.value < 0: + self.remove(item) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml new file mode 100644 index 0000000000..394abbf74d --- /dev/null +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -0,0 +1,56 @@ +# This file specifies rules for correctness according to how frappe doctype data model works. + +rules: +- id: frappe-modifying-after-submit + patterns: + - pattern: self.$ATTR = ... + - pattern-inside: | + def on_submit(self, ...): + ... + message: | + Doctype modified after submission. Please check if modification of self.$ATTR is commited to database. + languages: [python] + severity: ERROR + +- id: frappe-print-function-in-doctypes + pattern: print(...) + message: | + Did you mean to leave this print statement in? Consider using msgprint or logger instead of print statement. + languages: [python] + severity: WARNING + paths: + exclude: + - test_*.py + include: + - "*/**/doctype/*" + +- id: frappe-modifying-child-tables-while-iterating + pattern-either: + - pattern: | + for $ROW in self.$TABLE: + ... + self.remove(...) + - pattern: | + for $ROW in self.$TABLE: + ... + self.append(...) + message: | + Child table being modified while iterating on it. + languages: [python] + severity: ERROR + paths: + include: + - "*/**/doctype/*" + +- id: frappe-same-key-assigned-twice + pattern-either: + - pattern: | + {..., $X: $A, ..., $X: $B, ...} + - pattern: | + dict(..., ($X, $A), ..., ($X, $B), ...) + - pattern: | + _dict(..., ($X, $A), ..., ($X, $B), ...) + message: | + key `$X` is uselessly assigned twice. This could be a potential bug. + languages: [python] + severity: ERROR diff --git a/.github/helper/semgrep_rules/security.py b/.github/helper/semgrep_rules/security.py new file mode 100644 index 0000000000..f477d7c176 --- /dev/null +++ b/.github/helper/semgrep_rules/security.py @@ -0,0 +1,6 @@ +def function_name(input): + # ruleid: frappe-codeinjection-eval + eval(input) + +# ok: frappe-codeinjection-eval +eval("1 + 1") diff --git a/.github/helper/semgrep_rules/security.yml b/.github/helper/semgrep_rules/security.yml new file mode 100644 index 0000000000..5a5098bf50 --- /dev/null +++ b/.github/helper/semgrep_rules/security.yml @@ -0,0 +1,25 @@ +rules: +- id: frappe-codeinjection-eval + patterns: + - pattern-not: eval("...") + - pattern: eval(...) + message: | + Detected the use of eval(). eval() can be dangerous if used to evaluate + dynamic content. Avoid it or use safe_eval(). + languages: [python] + severity: ERROR + +- id: frappe-sqli-format-strings + patterns: + - pattern-inside: | + @frappe.whitelist() + def $FUNC(...): + ... + - pattern-either: + - pattern: frappe.db.sql("..." % ...) + - pattern: frappe.db.sql(f"...", ...) + - pattern: frappe.db.sql("...".format(...), ...) + message: | + Detected use of raw string formatting for SQL queries. This can lead to sql injection vulnerabilities. Refer security guidelines - https://github.com/frappe/erpnext/wiki/Code-Security-Guidelines + languages: [python] + severity: WARNING diff --git a/.github/helper/semgrep_rules/translate.js b/.github/helper/semgrep_rules/translate.js new file mode 100644 index 0000000000..7b92fe2dff --- /dev/null +++ b/.github/helper/semgrep_rules/translate.js @@ -0,0 +1,37 @@ +// ruleid: frappe-translation-empty-string +__("") +// ruleid: frappe-translation-empty-string +__('') + +// ok: frappe-translation-js-formatting +__('Welcome {0}, get started with ERPNext in just a few clicks.', [full_name]); + +// ruleid: frappe-translation-js-formatting +__(`Welcome ${full_name}, get started with ERPNext in just a few clicks.`); + +// ok: frappe-translation-js-formatting +__('This is fine'); + + +// ok: frappe-translation-trailing-spaces +__('This is fine'); + +// ruleid: frappe-translation-trailing-spaces +__(' this is not ok '); +// ruleid: frappe-translation-trailing-spaces +__('this is not ok '); +// ruleid: frappe-translation-trailing-spaces +__(' this is not ok'); + +// ok: frappe-translation-js-splitting +__('You have {0} subscribers in your mailing list.', [subscribers.length]) + +// todoruleid: frappe-translation-js-splitting +__('You have') + subscribers.length + __('subscribers in your mailing list.') + +// ruleid: frappe-translation-js-splitting +__('You have' + 'subscribers in your mailing list.') + +// ruleid: frappe-translation-js-splitting +__('You have {0} subscribers' + + 'in your mailing list', [subscribers.length]) diff --git a/.github/helper/semgrep_rules/translate.py b/.github/helper/semgrep_rules/translate.py new file mode 100644 index 0000000000..bd6cd9126c --- /dev/null +++ b/.github/helper/semgrep_rules/translate.py @@ -0,0 +1,53 @@ +# Examples taken from https://frappeframework.com/docs/user/en/translations +# This file is used for testing the tests. + +from frappe import _ + +full_name = "Jon Doe" +# ok: frappe-translation-python-formatting +_('Welcome {0}, get started with ERPNext in just a few clicks.').format(full_name) + +# ruleid: frappe-translation-python-formatting +_('Welcome %s, get started with ERPNext in just a few clicks.' % full_name) +# ruleid: frappe-translation-python-formatting +_('Welcome %(name)s, get started with ERPNext in just a few clicks.' % {'name': full_name}) + +# ruleid: frappe-translation-python-formatting +_('Welcome {0}, get started with ERPNext in just a few clicks.'.format(full_name)) + + +subscribers = ["Jon", "Doe"] +# ok: frappe-translation-python-formatting +_('You have {0} subscribers in your mailing list.').format(len(subscribers)) + +# ruleid: frappe-translation-python-splitting +_('You have') + len(subscribers) + _('subscribers in your mailing list.') + +# ruleid: frappe-translation-python-splitting +_('You have {0} subscribers \ + in your mailing list').format(len(subscribers)) + +# ok: frappe-translation-python-splitting +_('You have {0} subscribers') \ + + 'in your mailing list' + +# ruleid: frappe-translation-trailing-spaces +msg = _(" You have {0} pending invoice ") +# ruleid: frappe-translation-trailing-spaces +msg = _("You have {0} pending invoice ") +# ruleid: frappe-translation-trailing-spaces +msg = _(" You have {0} pending invoice") + +# ok: frappe-translation-trailing-spaces +msg = ' ' + _("You have {0} pending invoices") + ' ' + +# ruleid: frappe-translation-python-formatting +_(f"can not format like this - {subscribers}") +# ruleid: frappe-translation-python-splitting +_(f"what" + f"this is also not cool") + + +# ruleid: frappe-translation-empty-string +_("") +# ruleid: frappe-translation-empty-string +_('') diff --git a/.github/helper/semgrep_rules/translate.yml b/.github/helper/semgrep_rules/translate.yml new file mode 100644 index 0000000000..3737da5a7e --- /dev/null +++ b/.github/helper/semgrep_rules/translate.yml @@ -0,0 +1,63 @@ +rules: +- id: frappe-translation-empty-string + pattern-either: + - pattern: _("") + - pattern: __("") + message: | + Empty string is useless for translation. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [python, javascript, json] + severity: ERROR + +- id: frappe-translation-trailing-spaces + pattern-either: + - pattern: _("=~/(^[ \t]+|[ \t]+$)/") + - pattern: __("=~/(^[ \t]+|[ \t]+$)/") + message: | + Trailing or leading whitespace not allowed in translate strings. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [python, javascript, json] + severity: ERROR + +- id: frappe-translation-python-formatting + pattern-either: + - pattern: _("..." % ...) + - pattern: _("...".format(...)) + - pattern: _(f"...") + message: | + Only positional formatters are allowed and formatting should not be done before translating. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [python] + severity: ERROR + +- id: frappe-translation-js-formatting + patterns: + - pattern: __(`...`) + - pattern-not: __("...") + message: | + Template strings are not allowed for text formatting. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [javascript, json] + severity: ERROR + +- id: frappe-translation-python-splitting + pattern-either: + - pattern: _(...) + ... + _(...) + - pattern: _("..." + "...") + - pattern-regex: '_\([^\)]*\\\s*' + message: | + Do not split strings inside translate function. Do not concatenate using translate functions. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [python] + severity: ERROR + +- id: frappe-translation-js-splitting + pattern-either: + - pattern-regex: '__\([^\)]*[\+\\]\s*' + - pattern: __('...' + '...') + - pattern: __('...') + __('...') + message: | + Do not split strings inside translate function. Do not concatenate using translate functions. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [javascript, json] + severity: ERROR diff --git a/.github/helper/semgrep_rules/ux.py b/.github/helper/semgrep_rules/ux.py new file mode 100644 index 0000000000..4a74457435 --- /dev/null +++ b/.github/helper/semgrep_rules/ux.py @@ -0,0 +1,31 @@ +import frappe +from frappe import msgprint, throw, _ + + +# ruleid: frappe-missing-translate-function +throw("Error Occured") + +# ruleid: frappe-missing-translate-function +frappe.throw("Error Occured") + +# ruleid: frappe-missing-translate-function +frappe.msgprint("Useful message") + +# ruleid: frappe-missing-translate-function +msgprint("Useful message") + + +# ok: frappe-missing-translate-function +translatedmessage = _("Hello") + +# ok: frappe-missing-translate-function +throw(translatedmessage) + +# ok: frappe-missing-translate-function +msgprint(translatedmessage) + +# ok: frappe-missing-translate-function +msgprint(_("Helpful message")) + +# ok: frappe-missing-translate-function +frappe.throw(_("Error occured")) diff --git a/.github/helper/semgrep_rules/ux.yml b/.github/helper/semgrep_rules/ux.yml new file mode 100644 index 0000000000..ed06a6a80c --- /dev/null +++ b/.github/helper/semgrep_rules/ux.yml @@ -0,0 +1,15 @@ +rules: +- id: frappe-missing-translate-function + pattern-either: + - patterns: + - pattern: frappe.msgprint("...", ...) + - pattern-not: frappe.msgprint(_("..."), ...) + - pattern-not: frappe.msgprint(__("..."), ...) + - patterns: + - pattern: frappe.throw("...", ...) + - pattern-not: frappe.throw(_("..."), ...) + - pattern-not: frappe.throw(__("..."), ...) + 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, javascript, json] + severity: ERROR diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml new file mode 100644 index 0000000000..df08263236 --- /dev/null +++ b/.github/workflows/semgrep.yml @@ -0,0 +1,24 @@ +name: Semgrep + +on: + pull_request: + branches: + - develop +jobs: + semgrep: + name: Frappe Linter + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Setup python3 + uses: actions/setup-python@v2 + with: + python-version: 3.8 + - name: Run semgrep + run: | + python -m pip install -q semgrep + git fetch origin $GITHUB_BASE_REF:$GITHUB_BASE_REF -q + files=$(git diff --name-only --diff-filter=d $GITHUB_BASE_REF) + [[ -d .github/helper/semgrep_rules ]] && semgrep --severity ERROR --config=.github/helper/semgrep_rules --quiet --error $files + semgrep --config="r/python.lang.correctness" --quiet --error $files + [[ -d .github/helper/semgrep_rules ]] && semgrep --severity WARNING --severity INFO --config=.github/helper/semgrep_rules --quiet $files From d6be154ac27dcc69655213a0770610d81e438327 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 16 Apr 2021 22:08:44 +0530 Subject: [PATCH 13/31] fix: implicit string concatenation (#25371) * fix: implicit string concatenations * chore: rerun healthcare patch for company fields --- .../accounts/doctype/promotional_scheme/promotional_scheme.py | 4 ++-- erpnext/patches.txt | 2 +- .../patches/v13_0/set_company_field_in_healthcare_doctypes.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/promotional_scheme/promotional_scheme.py b/erpnext/accounts/doctype/promotional_scheme/promotional_scheme.py index 523e9ee08a..7d9302382f 100644 --- a/erpnext/accounts/doctype/promotional_scheme/promotional_scheme.py +++ b/erpnext/accounts/doctype/promotional_scheme/promotional_scheme.py @@ -9,7 +9,7 @@ from frappe.utils import cstr from frappe.model.naming import make_autoname from frappe.model.document import Document -pricing_rule_fields = ['apply_on', 'mixed_conditions', 'is_cumulative', 'other_item_code', 'other_item_group' +pricing_rule_fields = ['apply_on', 'mixed_conditions', 'is_cumulative', 'other_item_code', 'other_item_group', 'apply_rule_on_other', 'other_brand', 'selling', 'buying', 'applicable_for', 'valid_from', 'valid_upto', 'customer', 'customer_group', 'territory', 'sales_partner', 'campaign', 'supplier', 'supplier_group', 'company', 'currency', 'apply_multiple_pricing_rules'] @@ -111,4 +111,4 @@ def get_args_for_pricing_rule(doc): for d in pricing_rule_fields: args[d] = doc.get(d) - return args \ No newline at end of file + return args diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 1f800889c7..112f6d8a83 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -693,7 +693,7 @@ execute:frappe.reload_doctype('Dashboard') execute:frappe.reload_doc('desk', 'doctype', 'number_card_link') execute:frappe.delete_doc_if_exists('Dashboard', 'Accounts') erpnext.patches.v13_0.update_actual_start_and_end_date_in_wo -erpnext.patches.v13_0.set_company_field_in_healthcare_doctypes #2020-05-25 +erpnext.patches.v13_0.set_company_field_in_healthcare_doctypes #2021-04-16 erpnext.patches.v12_0.update_bom_in_so_mr execute:frappe.delete_doc("Report", "Department Analytics") execute:frappe.rename_doc("Desk Page", "Loan Management", "Loan", force=True) diff --git a/erpnext/patches/v13_0/set_company_field_in_healthcare_doctypes.py b/erpnext/patches/v13_0/set_company_field_in_healthcare_doctypes.py index be5e30f307..a5b93f6307 100644 --- a/erpnext/patches/v13_0/set_company_field_in_healthcare_doctypes.py +++ b/erpnext/patches/v13_0/set_company_field_in_healthcare_doctypes.py @@ -3,7 +3,7 @@ import frappe def execute(): company = frappe.db.get_single_value('Global Defaults', 'default_company') - doctypes = ['Clinical Procedure', 'Inpatient Record', 'Lab Test', 'Sample Collection' 'Patient Appointment', 'Patient Encounter', 'Vital Signs', 'Therapy Session', 'Therapy Plan', 'Patient Assessment'] + doctypes = ['Clinical Procedure', 'Inpatient Record', 'Lab Test', 'Sample Collection', 'Patient Appointment', 'Patient Encounter', 'Vital Signs', 'Therapy Session', 'Therapy Plan', 'Patient Assessment'] for entry in doctypes: if frappe.db.exists('DocType', entry): frappe.reload_doc('Healthcare', 'doctype', entry) From 18c7815a1b313c10cedf4135b51ecaeaa5c76bb7 Mon Sep 17 00:00:00 2001 From: Saqib Date: Sat, 17 Apr 2021 15:37:40 +0530 Subject: [PATCH 14/31] fix: presentation currency in statement of accounts (#25367) --- .../process_statement_of_accounts.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html index e1ddeff61f..94ae79a0c6 100644 --- a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html +++ b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.html @@ -38,22 +38,22 @@ {% endif %} - {{ frappe.utils.fmt_money(row.debit, filters.presentation_currency) }} + {{ frappe.utils.fmt_money(row.debit, currency=filters.presentation_currency) }} - {{ frappe.utils.fmt_money(row.credit, filters.presentation_currency) }} + {{ frappe.utils.fmt_money(row.credit, currency=filters.presentation_currency) }} {% else %} {{ frappe.format(row.account, {fieldtype: "Link"}) or " " }} - {{ row.account and frappe.utils.fmt_money(row.debit, filters.presentation_currency) }} + {{ row.account and frappe.utils.fmt_money(row.debit, currency=filters.presentation_currency) }} - {{ row.account and frappe.utils.fmt_money(row.credit, filters.presentation_currency) }} + {{ row.account and frappe.utils.fmt_money(row.credit, currency=filters.presentation_currency) }} {% endif %} - {{ frappe.utils.fmt_money(row.balance, filters.presentation_currency) }} + {{ frappe.utils.fmt_money(row.balance, currency=filters.presentation_currency) }} {% endfor %} From 75e13f7bb662d903dca2a4038ed489333604e34c Mon Sep 17 00:00:00 2001 From: Saqib Date: Sat, 17 Apr 2021 15:38:47 +0530 Subject: [PATCH 15/31] fix(e-invoicing): add company validation for e-invoicing (#25348) Co-authored-by: Nabin Hait --- .../doctype/sales_invoice/test_sales_invoice.py | 15 +++++++++++++-- erpnext/regional/india/e_invoice/utils.py | 7 ++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 4a6f9d1d6a..9059d0b040 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -1879,7 +1879,17 @@ class TestSalesInvoice(unittest.TestCase): def test_einvoice_submission_without_irn(self): # init - frappe.db.set_value('E Invoice Settings', 'E Invoice Settings', 'enable', 1) + einvoice_settings = frappe.get_doc('E Invoice Settings') + einvoice_settings.enable = 1 + einvoice_settings.applicable_from = nowdate() + einvoice_settings.append('credentials', { + 'company': '_Test Company', + 'gstin': '27AAECE4835E1ZR', + 'username': 'test', + 'password': 'test' + }) + einvoice_settings.save() + country = frappe.flags.country frappe.flags.country = 'India' @@ -1890,7 +1900,8 @@ class TestSalesInvoice(unittest.TestCase): si.submit() # reset - frappe.db.set_value('E Invoice Settings', 'E Invoice Settings', 'enable', 0) + einvoice_settings = frappe.get_doc('E Invoice Settings') + einvoice_settings.enable = 0 frappe.flags.country = country def test_einvoice_json(self): diff --git a/erpnext/regional/india/e_invoice/utils.py b/erpnext/regional/india/e_invoice/utils.py index 59c098c1ca..1d3cb661dd 100644 --- a/erpnext/regional/india/e_invoice/utils.py +++ b/erpnext/regional/india/e_invoice/utils.py @@ -38,12 +38,13 @@ def validate_eligibility(doc): einvoicing_eligible_from = frappe.db.get_single_value('E Invoice Settings', 'applicable_from') or '2021-04-01' if getdate(doc.get('posting_date')) < getdate(einvoicing_eligible_from): return False - + + invalid_company = not frappe.db.get_value('E Invoice User', { 'company': doc.get('company') }) invalid_supply_type = doc.get('gst_category') not in ['Registered Regular', 'SEZ', 'Overseas', 'Deemed Export'] company_transaction = doc.get('billing_address_gstin') == doc.get('company_gstin') no_taxes_applied = not doc.get('taxes') - if invalid_supply_type or company_transaction or no_taxes_applied: + if invalid_company or invalid_supply_type or company_transaction or no_taxes_applied: return False return True @@ -400,7 +401,7 @@ def validate_totals(einvoice): if abs(flt(value_details['AssVal']) - total_item_ass_value) > 1: frappe.throw(_('Total Taxable Value of the items is not equal to the Invoice Net Total. Please check item taxes / discounts for any correction.')) - if abs(flt(value_details['TotInvVal']) + flt(value_details['Discount']) - total_item_value) > 1: + if abs(flt(value_details['TotInvVal']) + flt(value_details['Discount']) - flt(value_details['OthChrg']) - total_item_value) > 1: frappe.throw(_('Total Value of the items is not equal to the Invoice Grand Total. Please check item taxes / discounts for any correction.')) calculated_invoice_value = \ From dedf2c1b61a8f31fb8da831e2c2a96611bacaa35 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 17 Apr 2021 14:57:57 +0530 Subject: [PATCH 16/31] fix: remove duplicate keys from dictionaries --- erpnext/accounts/doctype/sales_invoice/sales_invoice.py | 1 - erpnext/controllers/stock_controller.py | 1 - erpnext/manufacturing/dashboard_fixtures.py | 4 +--- .../manufacturing/doctype/production_plan/production_plan.py | 1 - erpnext/regional/report/gstr_1/gstr_1.py | 2 +- erpnext/stock/get_item_details.py | 2 -- erpnext/utilities/activation.py | 1 - 7 files changed, 2 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 3c91dccaa7..a731e79574 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -46,7 +46,6 @@ class SalesInvoice(SellingController): 'target_parent_dt': 'Sales Order', 'target_parent_field': 'per_billed', 'source_field': 'amount', - 'join_field': 'so_detail', 'percent_join_field': 'sales_order', 'status_field': 'billing_status', 'keyword': 'Billed', diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 20499579ca..34f7b27e00 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -117,7 +117,6 @@ class StockController(AccountsController): "account": expense_account, "against": warehouse_account[sle.warehouse]["account"], "cost_center": item_row.cost_center, - "project": item_row.project or self.get('project'), "remarks": self.get("remarks") or "Accounting Entry for Stock", "credit": flt(sle.stock_value_difference, precision), "project": item_row.get("project") or self.get("project"), diff --git a/erpnext/manufacturing/dashboard_fixtures.py b/erpnext/manufacturing/dashboard_fixtures.py index 0e9a21c026..7ba43d6471 100644 --- a/erpnext/manufacturing/dashboard_fixtures.py +++ b/erpnext/manufacturing/dashboard_fixtures.py @@ -43,7 +43,6 @@ def get_charts(): return [{ "doctype": "Dashboard Chart", "based_on": "modified", - "time_interval": "Yearly", "chart_type": "Sum", "chart_name": _("Produced Quantity"), "name": "Produced Quantity", @@ -60,7 +59,6 @@ def get_charts(): }, { "doctype": "Dashboard Chart", "based_on": "creation", - "time_interval": "Yearly", "chart_type": "Sum", "chart_name": _("Completed Operation"), "name": "Completed Operation", @@ -238,4 +236,4 @@ def get_number_cards(): "label": _("Monthly Quality Inspections"), "show_percentage_stats": 1, "stats_time_interval": "Weekly" - }] \ No newline at end of file + }] diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index cef2d8be7a..e7c83ac050 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -561,7 +561,6 @@ def get_material_request_items(row, sales_order, company, 'item_name': row.item_name, 'quantity': required_qty, 'required_bom_qty': total_qty, - 'description': row.description, 'stock_uom': row.get("stock_uom"), 'warehouse': warehouse or row.get('source_warehouse') \ or row.get('default_warehouse') or item_group_defaults.get("default_warehouse"), diff --git a/erpnext/regional/report/gstr_1/gstr_1.py b/erpnext/regional/report/gstr_1/gstr_1.py index 75076231c0..b637fb47b3 100644 --- a/erpnext/regional/report/gstr_1/gstr_1.py +++ b/erpnext/regional/report/gstr_1/gstr_1.py @@ -561,7 +561,7 @@ def get_json(filters, report_name, data): fp = "%02d%s" % (getdate(filters["to_date"]).month, getdate(filters["to_date"]).year) - gst_json = {"gstin": "", "version": "GST2.2.9", + gst_json = {"version": "GST2.2.9", "hash": "hash", "gstin": gstin, "fp": fp} res = {} diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index aaf14a535e..dedfe1d79b 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -309,8 +309,6 @@ def get_basic_details(args, item, overwrite_warehouse=True): "update_stock": args.get("update_stock") if args.get('doctype') in ['Sales Invoice', 'Purchase Invoice'] else 0, "delivered_by_supplier": item.delivered_by_supplier if args.get("doctype") in ["Sales Order", "Sales Invoice"] else 0, "is_fixed_asset": item.is_fixed_asset, - "weight_per_unit":item.weight_per_unit, - "weight_uom":item.weight_uom, "last_purchase_rate": item.last_purchase_rate if args.get("doctype") in ["Purchase Order"] else 0, "transaction_date": args.get("transaction_date"), "against_blanket_order": args.get("against_blanket_order"), diff --git a/erpnext/utilities/activation.py b/erpnext/utilities/activation.py index 7b17c8c464..50c4b255ce 100644 --- a/erpnext/utilities/activation.py +++ b/erpnext/utilities/activation.py @@ -18,7 +18,6 @@ def get_level(): "Delivery Note": 5, "Employee": 3, "Instructor": 5, - "Instructor": 5, "Issue": 5, "Item": 5, "Journal Entry": 3, From ad6a2657aee99b2d56afba99eca55bb09ef4f3d9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 17 Apr 2021 16:50:02 +0530 Subject: [PATCH 17/31] chore: minor translation fixes --- erpnext/accounts/doctype/sales_invoice/sales_invoice.py | 6 +++--- erpnext/controllers/stock_controller.py | 2 +- .../doctype/production_plan/production_plan.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index a731e79574..4461f29fe3 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -275,7 +275,7 @@ class SalesInvoice(SellingController): pluck="pos_closing_entry" ) if pos_closing_entry: - msg = _("To cancel a {} you need to cancel the POS Closing Entry {}. ").format( + msg = _("To cancel a {} you need to cancel the POS Closing Entry {}.").format( frappe.bold("Consolidated Sales Invoice"), get_link_to_form("POS Closing Entry", pos_closing_entry[0]) ) @@ -548,12 +548,12 @@ class SalesInvoice(SellingController): frappe.throw(_("Debit To is required"), title=_("Account Missing")) if account.report_type != "Balance Sheet": - msg = _("Please ensure {} account is a Balance Sheet account. ").format(frappe.bold("Debit To")) + msg = _("Please ensure {} account is a Balance Sheet account.").format(frappe.bold("Debit To")) + " " msg += _("You can change the parent account to a Balance Sheet account or select a different account.") frappe.throw(msg, title=_("Invalid Account")) if self.customer and account.account_type != "Receivable": - msg = _("Please ensure {} account is a Receivable account. ").format(frappe.bold("Debit To")) + msg = _("Please ensure {} account is a Receivable account.").format(frappe.bold("Debit To")) + " " msg += _("Change the account type to Receivable or select a different account.") frappe.throw(msg, title=_("Invalid Account")) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 34f7b27e00..b14c274515 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -482,7 +482,7 @@ class StockController(AccountsController): ) message += "

" rule_link = frappe.utils.get_link_to_form("Putaway Rule", rule) - message += _(" Please adjust the qty or edit {0} to proceed.").format(rule_link) + message += _("Please adjust the qty or edit {0} to proceed.").format(rule_link) return message def repost_future_sle_and_gle(self): diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index e7c83ac050..a3e23a6897 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -765,7 +765,7 @@ def get_items_for_material_requests(doc, warehouses=None): to_enable = frappe.bold(_("Ignore Existing Projected Quantity")) warehouse = frappe.bold(doc.get('for_warehouse')) message = _("As there are sufficient raw materials, Material Request is not required for Warehouse {0}.").format(warehouse) + "

" - message += _(" If you still want to proceed, please enable {0}.").format(to_enable) + message += _("If you still want to proceed, please enable {0}.").format(to_enable) frappe.msgprint(message, title=_("Note")) From 9229ee1745d864656ddee0b216a33078b7a932c6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 17 Apr 2021 15:41:10 +0530 Subject: [PATCH 18/31] fix: update shipment status in database Caught by semgrep rule: https://github.com/frappe/erpnext/blob/develop/.github/helper/semgrep_rules/frappe_correctness.yml#L4 --- erpnext/stock/doctype/shipment/shipment.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/shipment/shipment.py b/erpnext/stock/doctype/shipment/shipment.py index 4697a7b323..01fcee4cac 100644 --- a/erpnext/stock/doctype/shipment/shipment.py +++ b/erpnext/stock/doctype/shipment/shipment.py @@ -23,10 +23,10 @@ class Shipment(Document): frappe.throw(_('Please enter Shipment Parcel information')) if self.value_of_goods == 0: frappe.throw(_('Value of goods cannot be 0')) - self.status = 'Submitted' + self.db_set('status', 'Submitted') def on_cancel(self): - self.status = 'Cancelled' + self.db_set('status', 'Cancelled') def validate_weight(self): for parcel in self.shipment_parcel: From e972ceb79840783f3947a1856551d477154ec4be Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 17 Apr 2021 15:42:03 +0530 Subject: [PATCH 19/31] fix: patch for updating shipment status --- erpnext/patches.txt | 1 + erpnext/patches/v13_0/update_shipment_status.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 erpnext/patches/v13_0/update_shipment_status.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 112f6d8a83..620cc5be62 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -772,3 +772,4 @@ erpnext.patches.v12_0.purchase_receipt_status erpnext.patches.v13_0.fix_non_unique_represents_company erpnext.patches.v12_0.add_document_type_field_for_italy_einvoicing erpnext.patches.v13_0.make_non_standard_user_type #13-04-2021 +erpnext.patches.v13_0.update_shipment_status diff --git a/erpnext/patches/v13_0/update_shipment_status.py b/erpnext/patches/v13_0/update_shipment_status.py new file mode 100644 index 0000000000..c425599e26 --- /dev/null +++ b/erpnext/patches/v13_0/update_shipment_status.py @@ -0,0 +1,14 @@ +import frappe + +def execute(): + frappe.reload_doc("stock", "doctype", "shipment") + + # update submitted status + frappe.db.sql("""UPDATE `tabShipment` + SET status = "Submitted" + WHERE status = "Draft" AND docstatus = 1""") + + # update cancelled status + frappe.db.sql("""UPDATE `tabShipment` + SET status = "Cancelled" + WHERE status = "Draft" AND docstatus = 2""") From c28fcba77964edb57609fee17b0470638596e5a3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 17 Apr 2021 15:47:34 +0530 Subject: [PATCH 20/31] ci(semgrep): add correctness rule for on_cancel Changes done to doctype object in `on_submit` are not commited to database. Add rule to catch similar bugs. --- .../semgrep_rules/frappe_correctness.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index 394abbf74d..54df062480 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -7,11 +7,29 @@ rules: - pattern-inside: | def on_submit(self, ...): ... + - metavariable-regex: + metavariable: '$ATTR' + # this is negative look-ahead, add more attrs to ignore like (ignore|ignore_this_too|ignore_me) + regex: '^(?!status_updater)(.*)$' message: | Doctype modified after submission. Please check if modification of self.$ATTR is commited to database. languages: [python] severity: ERROR +- id: frappe-modifying-after-cancel + patterns: + - pattern: self.$ATTR = ... + - pattern-inside: | + def on_cancel(self, ...): + ... + - metavariable-regex: + metavariable: '$ATTR' + regex: '^(?!ignore_linked_doctypes|status_updater)(.*)$' + message: | + Doctype modified after cancellation. Please check if modification of self.$ATTR is commited to database. + languages: [python] + severity: ERROR + - id: frappe-print-function-in-doctypes pattern: print(...) message: | From 80d44cada4248946ed6e0cc8e61b09d2612b1547 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sun, 18 Apr 2021 18:33:34 +0200 Subject: [PATCH 21/31] fix: remove is_default from country wise tax --- erpnext/setup/setup_wizard/data/country_wise_tax.json | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/erpnext/setup/setup_wizard/data/country_wise_tax.json b/erpnext/setup/setup_wizard/data/country_wise_tax.json index 6305442ef2..5876488033 100644 --- a/erpnext/setup/setup_wizard/data/country_wise_tax.json +++ b/erpnext/setup/setup_wizard/data/country_wise_tax.json @@ -486,7 +486,6 @@ "sales_tax_templates": [ { "title": "Umsatzsteuer 19%", - "is_default": 1, "taxes": [ { "account_head": { @@ -513,7 +512,6 @@ "purchase_tax_templates": [ { "title": "Abziehbare Vorsteuer 19%", - "is_default": 1, "taxes": [ { "account_head": { @@ -567,7 +565,6 @@ "sales_tax_templates": [ { "title": "Umsatzsteuer 19%", - "is_default": 1, "taxes": [ { "account_head": { @@ -594,7 +591,6 @@ "purchase_tax_templates": [ { "title": "Abziehbare Vorsteuer 19%", - "is_default": 1, "taxes": [ { "account_head": { @@ -625,7 +621,6 @@ "sales_tax_templates": [ { "title": "Umsatzsteuer 19%", - "is_default": 1, "taxes": [ { "account_head": { @@ -652,7 +647,6 @@ "purchase_tax_templates": [ { "title": "Abziehbare Vorsteuer 19%", - "is_default": 1, "taxes": [ { "account_head": { @@ -683,7 +677,6 @@ "sales_tax_templates": [ { "title": "Umsatzsteuer 19%", - "is_default": 1, "taxes": [ { "account_head": { @@ -708,7 +701,6 @@ "purchase_tax_templates": [ { "title": "Abziehbare Vorsteuer 19%", - "is_default": 1, "taxes": [ { "account_head": { @@ -829,7 +821,6 @@ "item_tax_templates": [ { "title": "In State GST", - "is_default": 1, "taxes": [ { "tax_type": { @@ -893,7 +884,6 @@ "*": [ { "title": "In State GST", - "is_default": 1, "taxes": [ { "account_head": { From e78253152916ef8a16a467445d670dcd03bd6c83 Mon Sep 17 00:00:00 2001 From: Saqib Date: Mon, 19 Apr 2021 10:15:51 +0530 Subject: [PATCH 22/31] fix: Apply single transaction threshold on net_total instead of supplier credit amount (#25243) * fix: Apply single transaction threshold on net_total instead of supplier credit amount * fix: Apply single transaction threshold on net_total instead of supplier credit amount * fix: test Co-authored-by: Nabin Hait --- .../tax_withholding_category.py | 2 +- .../test_tax_withholding_category.py | 44 ------------------- 2 files changed, 1 insertion(+), 45 deletions(-) diff --git a/erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py b/erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py index 961bdb147f..09db7fee2b 100644 --- a/erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py +++ b/erpnext/accounts/doctype/tax_withholding_category/tax_withholding_category.py @@ -251,7 +251,7 @@ def get_tds_amount(ldc, parties, inv, tax_details, fiscal_year_details, tax_dedu threshold = tax_details.get('threshold', 0) cumulative_threshold = tax_details.get('cumulative_threshold', 0) - if ((threshold and supp_credit_amt >= threshold) or (cumulative_threshold and supp_credit_amt >= cumulative_threshold)): + if ((threshold and inv.net_total >= threshold) or (cumulative_threshold and supp_credit_amt >= cumulative_threshold)): if ldc and is_valid_certificate( ldc.valid_from, ldc.valid_upto, inv.posting_date, tax_deducted, diff --git a/erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py b/erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py index dd3b49aa04..0cea7612dd 100644 --- a/erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py +++ b/erpnext/accounts/doctype/tax_withholding_category/test_tax_withholding_category.py @@ -87,50 +87,6 @@ class TestTaxWithholdingCategory(unittest.TestCase): for d in invoices: d.cancel() - def test_single_threshold_tds_with_previous_vouchers(self): - invoices = [] - frappe.db.set_value("Supplier", "Test TDS Supplier2", "tax_withholding_category", "Single Threshold TDS") - pi = create_purchase_invoice(supplier="Test TDS Supplier2") - pi.submit() - invoices.append(pi) - - pi = create_purchase_invoice(supplier="Test TDS Supplier2") - pi.submit() - invoices.append(pi) - - self.assertEqual(pi.taxes_and_charges_deducted, 2000) - self.assertEqual(pi.grand_total, 8000) - - # delete invoices to avoid clashing - for d in invoices: - d.cancel() - - def test_single_threshold_tds_with_previous_vouchers_and_no_tds(self): - invoices = [] - doc = create_supplier(supplier_name = "Test TDS Supplier ABC", - tax_withholding_category="Single Threshold TDS") - supplier = doc.name - - pi = create_purchase_invoice(supplier=supplier) - pi.submit() - invoices.append(pi) - - # TDS not applied - pi = create_purchase_invoice(supplier=supplier, do_not_apply_tds=True) - pi.submit() - invoices.append(pi) - - pi = create_purchase_invoice(supplier=supplier) - pi.submit() - invoices.append(pi) - - self.assertEqual(pi.taxes_and_charges_deducted, 2000) - self.assertEqual(pi.grand_total, 8000) - - # delete invoices to avoid clashing - for d in invoices: - d.cancel() - def test_cumulative_threshold_tcs(self): frappe.db.set_value("Customer", "Test TCS Customer", "tax_withholding_category", "Cumulative Threshold TCS") invoices = [] From 7eac4a250d1d156859f9c0d18a0f45c8bcc5215d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 19 Apr 2021 10:33:39 +0530 Subject: [PATCH 23/31] fix: functions using mutable defaults (#25370) --- erpnext/controllers/queries.py | 4 +++- erpnext/controllers/status_updater.py | 4 +++- .../doctype/bom_update_tool/bom_update_tool.py | 6 ++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/erpnext/controllers/queries.py b/erpnext/controllers/queries.py index c0c13153de..bc1ac5ea06 100644 --- a/erpnext/controllers/queries.py +++ b/erpnext/controllers/queries.py @@ -713,7 +713,9 @@ def get_tax_template(doctype, txt, searchfield, start, page_len, filters): return [(d,) for d in set(taxes)] -def get_fields(doctype, fields=[]): +def get_fields(doctype, fields=None): + if fields is None: + fields = [] meta = frappe.get_meta(doctype) fields.extend(meta.get_search_fields()) diff --git a/erpnext/controllers/status_updater.py b/erpnext/controllers/status_updater.py index 0987d0985e..cdb6d244a6 100644 --- a/erpnext/controllers/status_updater.py +++ b/erpnext/controllers/status_updater.py @@ -371,10 +371,12 @@ class StatusUpdater(Document): ref_doc.db_set("per_billed", per_billed) ref_doc.set_status(update=True) -def get_allowance_for(item_code, item_allowance={}, global_qty_allowance=None, global_amount_allowance=None, qty_or_amount="qty"): +def get_allowance_for(item_code, item_allowance=None, global_qty_allowance=None, global_amount_allowance=None, qty_or_amount="qty"): """ Returns the allowance for the item, if not set, returns global allowance """ + if item_allowance is None: + item_allowance = {} if qty_or_amount == "qty": if item_allowance.get(item_code, frappe._dict()).get("qty"): return item_allowance[item_code].qty, item_allowance, global_qty_allowance, global_amount_allowance diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py index 742d18c4cd..8fbcd4ea1d 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py @@ -53,7 +53,9 @@ class BOMUpdateTool(Document): rate=%s, amount=stock_qty*%s where bom_no = %s and docstatus < 2 and parenttype='BOM'""", (self.new_bom, unit_cost, unit_cost, self.current_bom)) - def get_parent_boms(self, bom, bom_list=[]): + def get_parent_boms(self, bom, bom_list=None): + if bom_list is None: + bom_list = [] data = frappe.db.sql("""SELECT DISTINCT parent FROM `tabBOM Item` WHERE bom_no = %s AND docstatus < 2 AND parenttype='BOM'""", bom) @@ -106,4 +108,4 @@ def update_cost(): for bom in bom_list: frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) - frappe.db.auto_commit_on_many_writes = 0 \ No newline at end of file + frappe.db.auto_commit_on_many_writes = 0 From dcdd3bebbe3db01ee2987843d4bd4ca72cd913e5 Mon Sep 17 00:00:00 2001 From: Jannat Patel <31363128+pateljannat@users.noreply.github.com> Date: Mon, 19 Apr 2021 10:36:40 +0530 Subject: [PATCH 24/31] feat: Timer in LMS Quiz (#24246) * feat: new fields in quiz doctypes * feat: timer in lms quiz * fix: variable initialisation * fix: context, exception fix * fix:sider * fix:sider * fix: indentation * fix: timer * fix: sider * fix: return value and format * fix: show time taken only after all attempts are over * fix: sider Co-authored-by: pateljannat Co-authored-by: Marica --- .../course_enrollment/course_enrollment.py | 5 +- erpnext/education/doctype/quiz/quiz.json | 25 +- .../doctype/quiz_activity/quiz_activity.json | 423 ++---------------- erpnext/education/doctype/student/student.py | 2 +- erpnext/education/utils.py | 33 +- erpnext/public/js/education/lms/quiz.js | 72 ++- erpnext/www/lms/content.html | 55 ++- erpnext/www/lms/index.html | 12 +- erpnext/www/lms/topic.py | 2 +- 9 files changed, 218 insertions(+), 411 deletions(-) diff --git a/erpnext/education/doctype/course_enrollment/course_enrollment.py b/erpnext/education/doctype/course_enrollment/course_enrollment.py index f7aa6e9fc1..2b3acf1b93 100644 --- a/erpnext/education/doctype/course_enrollment/course_enrollment.py +++ b/erpnext/education/doctype/course_enrollment/course_enrollment.py @@ -41,7 +41,7 @@ class CourseEnrollment(Document): frappe.throw(_("Student is already enrolled via Course Enrollment {0}").format( get_link_to_form("Course Enrollment", enrollment)), title=_('Duplicate Entry')) - def add_quiz_activity(self, quiz_name, quiz_response, answers, score, status): + def add_quiz_activity(self, quiz_name, quiz_response, answers, score, status, time_taken): result = {k: ('Correct' if v else 'Wrong') for k,v in answers.items()} result_data = [] for key in answers: @@ -66,7 +66,8 @@ class CourseEnrollment(Document): "activity_date": frappe.utils.datetime.datetime.now(), "result": result_data, "score": score, - "status": status + "status": status, + "time_taken": time_taken }).insert(ignore_permissions = True) def add_activity(self, content_type, content): diff --git a/erpnext/education/doctype/quiz/quiz.json b/erpnext/education/doctype/quiz/quiz.json index 569c281f4c..16d7d7e4bf 100644 --- a/erpnext/education/doctype/quiz/quiz.json +++ b/erpnext/education/doctype/quiz/quiz.json @@ -1,4 +1,5 @@ { + "actions": [], "allow_import": 1, "allow_rename": 1, "autoname": "field:title", @@ -12,7 +13,10 @@ "quiz_configuration_section", "passing_score", "max_attempts", - "grading_basis" + "grading_basis", + "column_break_7", + "is_time_bound", + "duration" ], "fields": [ { @@ -58,9 +62,26 @@ "fieldtype": "Select", "label": "Grading Basis", "options": "Latest Highest Score\nLatest Attempt" + }, + { + "default": "0", + "fieldname": "is_time_bound", + "fieldtype": "Check", + "label": "Is Time-Bound" + }, + { + "depends_on": "is_time_bound", + "fieldname": "duration", + "fieldtype": "Duration", + "label": "Duration" + }, + { + "fieldname": "column_break_7", + "fieldtype": "Column Break" } ], - "modified": "2019-06-12 12:23:57.020508", + "links": [], + "modified": "2020-12-24 15:41:35.043262", "modified_by": "Administrator", "module": "Education", "name": "Quiz", diff --git a/erpnext/education/doctype/quiz_activity/quiz_activity.json b/erpnext/education/doctype/quiz_activity/quiz_activity.json index e78db42f7d..742c88754a 100644 --- a/erpnext/education/doctype/quiz_activity/quiz_activity.json +++ b/erpnext/education/doctype/quiz_activity/quiz_activity.json @@ -1,490 +1,163 @@ { - "allow_copy": 0, - "allow_events_in_timeline": 0, - "allow_guest_to_view": 0, - "allow_import": 0, - "allow_rename": 0, + "actions": [], "autoname": "format:EDU-QA-{YYYY}-{#####}", "beta": 1, "creation": "2018-10-15 15:48:40.482821", - "custom": 0, - "docstatus": 0, "doctype": "DocType", - "document_type": "", "editable_grid": 1, "engine": "InnoDB", + "field_order": [ + "enrollment", + "student", + "column_break_3", + "course", + "section_break_5", + "quiz", + "column_break_7", + "status", + "section_break_9", + "result", + "section_break_11", + "activity_date", + "score", + "column_break_14", + "time_taken" + ], "fields": [ { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "enrollment", "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": "Enrollment", - "length": 0, - "no_copy": 0, "options": "Course Enrollment", - "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": 1, - "translatable": 0, - "unique": 0 + "set_only_once": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fetch_from": "enrollment.student", "fieldname": "student", "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": "Student", - "length": 0, - "no_copy": 0, "options": "Student", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "read_only": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "column_break_3", - "fieldtype": "Column Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "length": 0, - "no_copy": 0, - "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 + "fieldtype": "Column Break" }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fetch_from": "enrollment.course", "fieldname": "course", "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": "Course", - "length": 0, - "no_copy": 0, "options": "Course", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 1, - "translatable": 0, - "unique": 0 + "set_only_once": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "section_break_5", - "fieldtype": "Section Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "length": 0, - "no_copy": 0, - "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 + "fieldtype": "Section Break" }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "quiz", "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": "Quiz", - "length": 0, - "no_copy": 0, "options": "Quiz", - "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": 1, - "translatable": 0, - "unique": 0 + "set_only_once": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "column_break_7", - "fieldtype": "Column Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "length": 0, - "no_copy": 0, - "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 + "fieldtype": "Column Break" }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "status", "fieldtype": "Select", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, "label": "Status", - "length": 0, - "no_copy": 0, "options": "\nPass\nFail", - "permlevel": 0, - "precision": "", - "print_hide": 0, - "print_hide_if_no_value": 0, - "read_only": 1, - "remember_last_selected_value": 0, - "report_hide": 0, - "reqd": 0, - "search_index": 0, - "set_only_once": 0, - "translatable": 0, - "unique": 0 + "read_only": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "section_break_9", - "fieldtype": "Section Break", - "hidden": 0, - "ignore_user_permissions": 0, - "ignore_xss_filter": 0, - "in_filter": 0, - "in_global_search": 0, - "in_list_view": 0, - "in_standard_filter": 0, - "length": 0, - "no_copy": 0, - "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 + "fieldtype": "Section Break" }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "result", "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": "Result", - "length": 0, - "no_copy": 0, "options": "Quiz Result", - "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": 1, - "translatable": 0, - "unique": 0 + "set_only_once": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "activity_date", "fieldtype": "Data", - "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": "Activity Date", - "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": 1, - "translatable": 0, - "unique": 0 + "set_only_once": 1 }, { - "allow_bulk_edit": 0, - "allow_in_quick_entry": 0, - "allow_on_submit": 0, - "bold": 0, - "collapsible": 0, - "columns": 0, "fieldname": "score", "fieldtype": "Data", - "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": "Score", - "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": 1, - "translatable": 0, - "unique": 0 + "set_only_once": 1 + }, + { + "fieldname": "time_taken", + "fieldtype": "Duration", + "label": "Time Taken", + "set_only_once": 1 + }, + { + "fieldname": "section_break_11", + "fieldtype": "Section Break" + }, + { + "fieldname": "column_break_14", + "fieldtype": "Column Break" } ], - "has_web_view": 0, - "hide_heading": 0, - "hide_toolbar": 0, - "idx": 0, - "image_view": 0, - "in_create": 0, - "is_submittable": 0, - "issingle": 0, - "istable": 0, - "max_attachments": 0, - "modified": "2018-11-25 19:05:52.434437", + "links": [], + "modified": "2020-12-24 15:41:20.085380", "modified_by": "Administrator", "module": "Education", "name": "Quiz Activity", - "name_case": "", "owner": "Administrator", "permissions": [ { - "amend": 0, - "cancel": 0, "create": 1, "delete": 1, "email": 1, "export": 1, - "if_owner": 0, - "import": 0, - "permlevel": 0, "print": 1, "read": 1, "report": 1, "role": "Academics User", - "set_user_permissions": 0, "share": 1, - "submit": 0, "write": 1 }, { - "amend": 0, - "cancel": 0, "create": 1, "delete": 1, "email": 1, "export": 1, - "if_owner": 0, - "import": 0, - "permlevel": 0, "print": 1, "read": 1, "report": 1, "role": "LMS User", - "set_user_permissions": 0, "share": 1, - "submit": 0, "write": 1 }, { - "amend": 0, - "cancel": 0, - "create": 0, - "delete": 0, "email": 1, "export": 1, - "if_owner": 0, - "import": 0, - "permlevel": 0, "print": 1, "read": 1, "report": 1, "role": "Instructor", - "set_user_permissions": 0, - "share": 1, - "submit": 0, - "write": 0 + "share": 1 } ], "quick_entry": 1, - "read_only": 0, - "read_only_onload": 0, - "show_name_in_global_search": 0, "sort_field": "modified", "sort_order": "DESC", - "track_changes": 1, - "track_seen": 0, - "track_views": 0 + "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/education/doctype/student/student.py b/erpnext/education/doctype/student/student.py index 81626f1918..2dc0f634f0 100644 --- a/erpnext/education/doctype/student/student.py +++ b/erpnext/education/doctype/student/student.py @@ -114,7 +114,7 @@ class Student(Document): status = check_content_completion(content.name, content.doctype, course_enrollment_name) progress.append({'content': content.name, 'content_type': content.doctype, 'is_complete': status}) elif content.doctype == 'Quiz': - status, score, result = check_quiz_completion(content, course_enrollment_name) + status, score, result, time_taken = check_quiz_completion(content, course_enrollment_name) progress.append({'content': content.name, 'content_type': content.doctype, 'is_complete': status, 'score': score, 'result': result}) return progress diff --git a/erpnext/education/utils.py b/erpnext/education/utils.py index cffc3960a0..8f51fef847 100644 --- a/erpnext/education/utils.py +++ b/erpnext/education/utils.py @@ -194,7 +194,7 @@ def add_activity(course, content_type, content, program): return enrollment.add_activity(content_type, content) @frappe.whitelist() -def evaluate_quiz(quiz_response, quiz_name, course, program): +def evaluate_quiz(quiz_response, quiz_name, course, program, time_taken): import json student = get_current_student() @@ -209,7 +209,7 @@ def evaluate_quiz(quiz_response, quiz_name, course, program): if student: enrollment = get_or_create_course_enrollment(course, program) if quiz.allowed_attempt(enrollment, quiz_name): - enrollment.add_quiz_activity(quiz_name, quiz_response, result, score, status) + enrollment.add_quiz_activity(quiz_name, quiz_response, result, score, status, time_taken) return {'result': result, 'score': score, 'status': status} else: return None @@ -219,8 +219,9 @@ def get_quiz(quiz_name, course): try: quiz = frappe.get_doc("Quiz", quiz_name) questions = quiz.get_questions() + duration = quiz.duration except: - frappe.throw(_("Quiz {0} does not exist").format(quiz_name)) + frappe.throw(_("Quiz {0} does not exist").format(quiz_name), frappe.DoesNotExistError) return None questions = [{ @@ -232,12 +233,20 @@ def get_quiz(quiz_name, course): } for question in questions] if has_super_access(): - return {'questions': questions, 'activity': None} + return { + 'questions': questions, + 'activity': None, + 'duration':duration + } student = get_current_student() course_enrollment = get_enrollment("course", course, student.name) - status, score, result = check_quiz_completion(quiz, course_enrollment) - return {'questions': questions, 'activity': {'is_complete': status, 'score': score, 'result': result}} + status, score, result, time_taken = check_quiz_completion(quiz, course_enrollment) + return { + 'questions': questions, + 'activity': {'is_complete': status, 'score': score, 'result': result, 'time_taken': time_taken}, + 'duration': quiz.duration + } def get_topic_progress(topic, course_name, program): """ @@ -361,15 +370,23 @@ def check_content_completion(content_name, content_type, enrollment_name): return False def check_quiz_completion(quiz, enrollment_name): - attempts = frappe.get_all("Quiz Activity", filters={'enrollment': enrollment_name, 'quiz': quiz.name}, fields=["name", "activity_date", "score", "status"]) + attempts = frappe.get_all("Quiz Activity", + filters={ + 'enrollment': enrollment_name, + 'quiz': quiz.name + }, + fields=["name", "activity_date", "score", "status", "time_taken"] + ) status = False if quiz.max_attempts == 0 else bool(len(attempts) >= quiz.max_attempts) score = None result = None + time_taken = None if attempts: if quiz.grading_basis == 'Last Highest Score': attempts = sorted(attempts, key = lambda i: int(i.score), reverse=True) score = attempts[0]['score'] result = attempts[0]['status'] + time_taken = attempts[0]['time_taken'] if result == 'Pass': status = True - return status, score, result \ No newline at end of file + return status, score, result, time_taken \ No newline at end of file diff --git a/erpnext/public/js/education/lms/quiz.js b/erpnext/public/js/education/lms/quiz.js index 4a9d1e34e6..32fa4ab1ec 100644 --- a/erpnext/public/js/education/lms/quiz.js +++ b/erpnext/public/js/education/lms/quiz.js @@ -20,6 +20,16 @@ class Quiz { } make(data) { + if (data.duration) { + const timer_display = document.createElement("div"); + timer_display.classList.add("lms-timer", "float-right", "font-weight-bold"); + document.getElementsByClassName("lms-title")[0].appendChild(timer_display); + if (!data.activity || (data.activity && !data.activity.is_complete)) { + this.initialiseTimer(data.duration); + this.is_time_bound = true; + this.time_taken = 0; + } + } data.questions.forEach(question_data => { let question_wrapper = document.createElement('div'); let question = new Question({ @@ -37,12 +47,51 @@ class Quiz { indicator = 'green' message = 'You have already cleared the quiz.' } - + if (data.activity.time_taken) { + this.calculate_and_display_time(data.activity.time_taken, "Time Taken - "); + } this.set_quiz_footer(message, indicator, data.activity.score) } else { this.make_actions(); } + window.addEventListener('beforeunload', (event) => { + event.preventDefault(); + event.returnValue = ''; + }); + } + + initialiseTimer(duration) { + this.time_left = duration; + var self = this; + var old_diff; + this.calculate_and_display_time(this.time_left, "Time Left - "); + this.start_time = new Date().getTime(); + this.timer = setInterval(function () { + var diff = (new Date().getTime() - self.start_time)/1000; + var variation = old_diff ? diff - old_diff : diff; + old_diff = diff; + self.time_left -= variation; + self.time_taken += variation; + self.calculate_and_display_time(self.time_left, "Time Left - "); + if (self.time_left <= 0) { + clearInterval(self.timer); + self.time_taken -= 1; + self.submit(); + } + }, 1000); + } + + calculate_and_display_time(second, text) { + var timer_display = document.getElementsByClassName("lms-timer")[0]; + var hours = this.append_zero(Math.floor(second / 3600)); + var minutes = this.append_zero(Math.floor(second % 3600 / 60)); + var seconds = this.append_zero(Math.ceil(second % 3600 % 60)); + timer_display.innerText = text + hours + ":" + minutes + ":" + seconds; + } + + append_zero(time) { + return time > 9 ? time : "0" + time; } make_actions() { @@ -57,6 +106,10 @@ class Quiz { } submit() { + if (this.is_time_bound) { + clearInterval(this.timer); + $(".lms-timer").text(""); + } this.submit_btn.innerText = 'Evaluating..' this.submit_btn.disabled = true this.disable() @@ -64,7 +117,8 @@ class Quiz { quiz_name: this.name, quiz_response: this.get_selected(), course: this.course, - program: this.program + program: this.program, + time_taken: this.is_time_bound ? this.time_taken : "" }).then(res => { this.submit_btn.remove() if (!res.message) { @@ -157,7 +211,7 @@ class Question { return input; } - let make_label = function(name, value) { + let make_label = function (name, value) { let label = document.createElement('label'); label.classList.add('form-check-label'); label.htmlFor = name; @@ -166,14 +220,14 @@ class Question { } let make_option = function (wrapper, option) { - let option_div = document.createElement('div') - option_div.classList.add('form-check', 'pb-1') + let option_div = document.createElement('div'); + option_div.classList.add('form-check', 'pb-1'); let input = make_input(option.name, option.option); let label = make_label(option.name, option.option); - option_div.appendChild(input) - option_div.appendChild(label) - wrapper.appendChild(option_div) - return {input: input, ...option} + option_div.appendChild(input); + option_div.appendChild(label); + wrapper.appendChild(option_div); + return { input: input, ...option }; } let options_wrapper = document.createElement('div') diff --git a/erpnext/www/lms/content.html b/erpnext/www/lms/content.html index dc9b6d80fb..15afb097b9 100644 --- a/erpnext/www/lms/content.html +++ b/erpnext/www/lms/content.html @@ -62,7 +62,7 @@ {{_('Back to Course')}} -
+

{{ content.name }} ({{ position + 1 }}/{{length}})

{% endmacro %} @@ -169,14 +169,51 @@ const next_url = '/lms/course?name={{ course }}&program={{ program }}' {% endif %} frappe.ready(() => { - const quiz = new Quiz(document.getElementById('quiz-wrapper'), { - name: '{{ content.name }}', - course: '{{ course }}', - program: '{{ program }}', - quiz_exit_button: quiz_exit_button, - next_url: next_url - }) - window.quiz = quiz; + {% if content.is_time_bound %} + var duration = get_duration("{{content.duration}}") + var d = frappe.msgprint({ + title: __('Important Notice'), + indicator: "red", + message: __(`This is a Time-Bound Quiz.

+ A timer for ${duration} will start, once you click on Proceed.

+ If you fail to submit before the time is up, the Quiz will be submitted automatically.`), + primary_action: { + label: __("Proceed"), + action: () => { + create_quiz(); + d.hide(); + } + }, + secondary_action: { + action: () => { + d.hide(); + window.location.href = "/lms/course?name={{ course }}&program={{ program }}"; + }, + label: __("Go Back"), + } + }); + {% else %} + create_quiz(); + {% endif %} + function create_quiz() { + const quiz = new Quiz(document.getElementById('quiz-wrapper'), { + name: '{{ content.name }}', + course: '{{ course }}', + program: '{{ program }}', + quiz_exit_button: quiz_exit_button, + next_url: next_url + }) + window.quiz = quiz; + } + function get_duration(seconds){ + var hours = append_zero(Math.floor(seconds / 3600)); + var minutes = append_zero(Math.floor(seconds % 3600 / 60)); + var seconds = append_zero(Math.floor(seconds % 3600 % 60)); + return `${hours}:${minutes}:${seconds}`; + } + function append_zero(time) { + return time > 9 ? time : "0" + time; + } }) {% endif %} diff --git a/erpnext/www/lms/index.html b/erpnext/www/lms/index.html index 7b239acd56..c1e96205eb 100644 --- a/erpnext/www/lms/index.html +++ b/erpnext/www/lms/index.html @@ -42,7 +42,9 @@

{{ education_settings.portal_title }}

-

{{ education_settings.description }}

+ {% if education_settings.description %} +

{{ education_settings.description }}

+ {% endif %}

{% if frappe.session.user == 'Guest' %} {{_('Sign Up')}} @@ -51,13 +53,15 @@

- {% for program in featured_programs %} - {{ program_card(program.program, program.has_access) }} - {% endfor %} {% if featured_programs %} + {% for program in featured_programs %} + {{ program_card(program.program, program.has_access) }} + {% endfor %} {% for n in range( (3 - (featured_programs|length)) %3) %} {{ null_card() }} {% endfor %} + {% else %} +

You have not enrolled in any program. Contact your Instructor.

{% endif %}
diff --git a/erpnext/www/lms/topic.py b/erpnext/www/lms/topic.py index f75ae8e9b6..8abbc72e91 100644 --- a/erpnext/www/lms/topic.py +++ b/erpnext/www/lms/topic.py @@ -35,7 +35,7 @@ def get_contents(topic, course, program): progress.append({'content': content, 'content_type': content.doctype, 'completed': status}) elif content.doctype == 'Quiz': if student: - status, score, result = utils.check_quiz_completion(content, course_enrollment.name) + status, score, result, time_taken = utils.check_quiz_completion(content, course_enrollment.name) else: status = False score = None From e8bc912ffcafeccfd41c8944f6ac053a68d9ac56 Mon Sep 17 00:00:00 2001 From: Marica Date: Mon, 19 Apr 2021 11:05:21 +0530 Subject: [PATCH 25/31] perf: Fetching exchange rate on every line item slows down PO (#25345) * fix: Dont fetch exchange rates for each line item once fetched at parent ` * perf: Use price list conversion rate from parent - If price list conversion rate exists in args already from earlier call, use that - `get_price_list_currency_and_exchange_rate` wont be called for each child row Co-authored-by: Nabin Hait --- erpnext/public/js/controllers/transaction.js | 2 ++ erpnext/stock/get_item_details.py | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 6c2144d6cb..a0398e718f 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -1103,6 +1103,8 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ to_currency: to_currency, args: args }, + freeze: true, + freeze_message: __("Fetching exchange rates ..."), callback: function(r) { callback(flt(r.message)); } diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index dedfe1d79b..1a61f30b9a 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -609,8 +609,12 @@ def get_price_list_rate(args, item_doc, out): meta = frappe.get_meta(args.parenttype or args.doctype) if meta.get_field("currency") or args.get('currency'): - pl_details = get_price_list_currency_and_exchange_rate(args) - args.update(pl_details) + if not args.get("price_list_currency") or not args.get("plc_conversion_rate"): + # if currency and plc_conversion_rate exist then + # `get_price_list_currency_and_exchange_rate` has already been called + pl_details = get_price_list_currency_and_exchange_rate(args) + args.update(pl_details) + if meta.get_field("currency"): validate_conversion_rate(args, meta) @@ -1000,6 +1004,8 @@ def apply_price_list(args, as_doc=False): args = process_args(args) parent = get_price_list_currency_and_exchange_rate(args) + args.update(parent) + children = [] if "items" in args: @@ -1064,7 +1070,7 @@ def get_price_list_currency_and_exchange_rate(args): return frappe._dict({ "price_list_currency": price_list_currency, "price_list_uom_dependant": price_list_uom_dependant, - "plc_conversion_rate": plc_conversion_rate + "plc_conversion_rate": plc_conversion_rate or 1 }) @frappe.whitelist() From 9c9907cf8e26ea65d330f1fec07d01afa2038017 Mon Sep 17 00:00:00 2001 From: Afshan <33727827+AfshanKhan@users.noreply.github.com> Date: Mon, 19 Apr 2021 11:48:28 +0530 Subject: [PATCH 26/31] fix: available employee for selection (#25377) * fix: available employee for selection * fix: available employee for selection fix: available employee for selection --- .../doctype/payroll_entry/payroll_entry.js | 4 + .../doctype/payroll_entry/payroll_entry.py | 153 ++++++++++-------- 2 files changed, 88 insertions(+), 69 deletions(-) diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js index 85bb651af7..f2892600d1 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.js +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.js @@ -151,6 +151,10 @@ frappe.ui.form.on('Payroll Entry', { filters['company'] = frm.doc.company; filters['start_date'] = frm.doc.start_date; filters['end_date'] = frm.doc.end_date; + filters['salary_slip_based_on_timesheet'] = frm.doc.salary_slip_based_on_timesheet; + filters['payroll_frequency'] = frm.doc.payroll_frequency; + filters['payroll_payable_account'] = frm.doc.payroll_payable_account; + filters['currency'] = frm.doc.currency; if (frm.doc.department) { filters['department'] = frm.doc.department; diff --git a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py index 4c9469e277..3953b463f1 100644 --- a/erpnext/payroll/doctype/payroll_entry/payroll_entry.py +++ b/erpnext/payroll/doctype/payroll_entry/payroll_entry.py @@ -52,49 +52,32 @@ class PayrollEntry(Document): Returns list of active employees based on selected criteria and for which salary structure exists """ - cond = self.get_filter_condition() - cond += self.get_joining_relieving_condition() + self.check_mandatory() + filters = self.make_filters() + cond = get_filter_condition(filters) + cond += get_joining_relieving_condition(self.start_date, self.end_date) condition = '' if self.payroll_frequency: condition = """and payroll_frequency = '%(payroll_frequency)s'"""% {"payroll_frequency": self.payroll_frequency} - sal_struct = frappe.db.sql_list(""" - select - name from `tabSalary Structure` - where - docstatus = 1 and - is_active = 'Yes' - and company = %(company)s - and currency = %(currency)s and - ifnull(salary_slip_based_on_timesheet,0) = %(salary_slip_based_on_timesheet)s - {condition}""".format(condition=condition), - {"company": self.company, "currency": self.currency, "salary_slip_based_on_timesheet":self.salary_slip_based_on_timesheet}) - + sal_struct = get_sal_struct(self.company, self.currency, self.salary_slip_based_on_timesheet, condition) if sal_struct: cond += "and t2.salary_structure IN %(sal_struct)s " cond += "and t2.payroll_payable_account = %(payroll_payable_account)s " cond += "and %(from_date)s >= t2.from_date" - emp_list = frappe.db.sql(""" - select - distinct t1.name as employee, t1.employee_name, t1.department, t1.designation - from - `tabEmployee` t1, `tabSalary Structure Assignment` t2 - where - t1.name = t2.employee - and t2.docstatus = 1 - %s order by t2.from_date desc - """ % cond, {"sal_struct": tuple(sal_struct), "from_date": self.end_date, "payroll_payable_account": self.payroll_payable_account}, as_dict=True) - - emp_list = self.remove_payrolled_employees(emp_list) + emp_list = get_emp_list(sal_struct, cond, self.end_date, self.payroll_payable_account) + emp_list = remove_payrolled_employees(emp_list, self.start_date, self.end_date) return emp_list - def remove_payrolled_employees(self, emp_list): - for employee_details in emp_list: - if frappe.db.exists("Salary Slip", {"employee": employee_details.employee, "start_date": self.start_date, "end_date": self.end_date, "docstatus": 1}): - emp_list.remove(employee_details) + def make_filters(self): + filters = frappe._dict() + filters['company'] = self.company + filters['branch'] = self.branch + filters['department'] = self.department + filters['designation'] = self.designation - return emp_list + return filters @frappe.whitelist() def fill_employee_details(self): @@ -122,23 +105,6 @@ class PayrollEntry(Document): if self.validate_attendance: return self.validate_employee_attendance() - def get_filter_condition(self): - self.check_mandatory() - - cond = '' - for f in ['company', 'branch', 'department', 'designation']: - if self.get(f): - cond += " and t1." + f + " = " + frappe.db.escape(self.get(f)) - - return cond - - def get_joining_relieving_condition(self): - cond = """ - and ifnull(t1.date_of_joining, '0000-00-00') <= '%(end_date)s' - and ifnull(t1.relieving_date, '2199-12-31') >= '%(start_date)s' - """ % {"start_date": self.start_date, "end_date": self.end_date} - return cond - def check_mandatory(self): for fieldname in ['company', 'start_date', 'end_date']: if not self.get(fieldname): @@ -451,6 +417,53 @@ class PayrollEntry(Document): marked_days = attendances[0][0] return marked_days +def get_sal_struct(company, currency, salary_slip_based_on_timesheet, condition): + return frappe.db.sql_list(""" + select + name from `tabSalary Structure` + where + docstatus = 1 and + is_active = 'Yes' + and company = %(company)s + and currency = %(currency)s and + ifnull(salary_slip_based_on_timesheet,0) = %(salary_slip_based_on_timesheet)s + {condition}""".format(condition=condition), + {"company": company, "currency": currency, "salary_slip_based_on_timesheet": salary_slip_based_on_timesheet}) + +def get_filter_condition(filters): + cond = '' + for f in ['company', 'branch', 'department', 'designation']: + if filters.get(f): + cond += " and t1." + f + " = " + frappe.db.escape(filters.get(f)) + + return cond + +def get_joining_relieving_condition(start_date, end_date): + cond = """ + and ifnull(t1.date_of_joining, '0000-00-00') <= '%(end_date)s' + and ifnull(t1.relieving_date, '2199-12-31') >= '%(start_date)s' + """ % {"start_date": start_date, "end_date": end_date} + return cond + +def get_emp_list(sal_struct, cond, end_date, payroll_payable_account): + return frappe.db.sql(""" + select + distinct t1.name as employee, t1.employee_name, t1.department, t1.designation + from + `tabEmployee` t1, `tabSalary Structure Assignment` t2 + where + t1.name = t2.employee + and t2.docstatus = 1 + %s order by t2.from_date desc + """ % cond, {"sal_struct": tuple(sal_struct), "from_date": end_date, "payroll_payable_account": payroll_payable_account}, as_dict=True) + +def remove_payrolled_employees(emp_list, start_date, end_date): + for employee_details in emp_list: + if frappe.db.exists("Salary Slip", {"employee": employee_details.employee, "start_date": start_date, "end_date": end_date, "docstatus": 1}): + emp_list.remove(employee_details) + + return emp_list + @frappe.whitelist() def get_start_end_dates(payroll_frequency, start_date=None, company=None): '''Returns dict of start and end dates for given payroll frequency based on start_date''' @@ -639,39 +652,41 @@ def get_payroll_entries_for_jv(doctype, txt, searchfield, start, page_len, filte 'start': start, 'page_len': page_len }) -def get_employee_with_existing_salary_slip(start_date, end_date, company): - return frappe.db.sql_list(""" - select employee from `tabSalary Slip` - where - (start_date between %(start_date)s and %(end_date)s - or - end_date between %(start_date)s and %(end_date)s - or - %(start_date)s between start_date and end_date) - and company = %(company)s - and docstatus = 1 - """, {'start_date': start_date, 'end_date': end_date, 'company': company}) +def get_employee_list(filters): + cond = get_filter_condition(filters) + cond += get_joining_relieving_condition(filters.start_date, filters.end_date) + condition = """and payroll_frequency = '%(payroll_frequency)s'"""% {"payroll_frequency": filters.payroll_frequency} + sal_struct = get_sal_struct(filters.company, filters.currency, filters.salary_slip_based_on_timesheet, condition) + if sal_struct: + cond += "and t2.salary_structure IN %(sal_struct)s " + cond += "and t2.payroll_payable_account = %(payroll_payable_account)s " + cond += "and %(from_date)s >= t2.from_date" + emp_list = get_emp_list(sal_struct, cond, filters.end_date, filters.payroll_payable_account) + emp_list = remove_payrolled_employees(emp_list, filters.start_date, filters.end_date) + return emp_list @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def employee_query(doctype, txt, searchfield, start, page_len, filters): filters = frappe._dict(filters) conditions = [] - exclude_employees = [] + include_employees = [] emp_cond = '' if filters.start_date and filters.end_date: - employee_list = get_employee_with_existing_salary_slip(filters.start_date, filters.end_date, filters.company) + employee_list = get_employee_list(filters) emp = filters.get('employees') + include_employees = [employee.employee for employee in employee_list if employee.employee not in emp] filters.pop('start_date') filters.pop('end_date') + filters.pop('salary_slip_based_on_timesheet') + filters.pop('payroll_frequency') + filters.pop('payroll_payable_account') + filters.pop('currency') if filters.employees is not None: filters.pop('employees') - if employee_list: - exclude_employees.extend(employee_list) - if emp: - exclude_employees.extend(emp) - if exclude_employees: - emp_cond += 'and employee not in %(exclude_employees)s' + + if include_employees: + emp_cond += 'and employee in %(include_employees)s' return frappe.db.sql("""select name, employee_name from `tabEmployee` where status = 'Active' @@ -695,4 +710,4 @@ def employee_query(doctype, txt, searchfield, start, page_len, filters): '_txt': txt.replace("%", ""), 'start': start, 'page_len': page_len, - 'exclude_employees': exclude_employees}) + 'include_employees': include_employees}) From 6c88ab07c779de864a88d9c073a1092b696f51c8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 19 Apr 2021 11:51:46 +0530 Subject: [PATCH 27/31] fix: commit leave_allocation change to db (#25382) --- .../compensatory_leave_request/compensatory_leave_request.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py b/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py index aa5a67f40c..a6fe429be1 100644 --- a/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py +++ b/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py @@ -66,7 +66,7 @@ class CompensatoryLeaveRequest(Document): else: leave_allocation = self.create_leave_allocation(leave_period, date_difference) - self.leave_allocation=leave_allocation.name + self.db_set("leave_allocation", leave_allocation.name) else: frappe.throw(_("There is no leave period in between {0} and {1}").format(format_date(self.work_from_date), format_date(self.work_end_date))) @@ -124,4 +124,4 @@ class CompensatoryLeaveRequest(Document): )) allocation.insert(ignore_permissions=True) allocation.submit() - return allocation \ No newline at end of file + return allocation From ac8a467b0a5694a86a992d3c8e1b14b362e19a57 Mon Sep 17 00:00:00 2001 From: Alan <2.alan.tom@gmail.com> Date: Mon, 19 Apr 2021 12:38:25 +0530 Subject: [PATCH 28/31] fix: exclude spurious Stock Entry Types from 'consumed' calculation (#25352) * fix: exclude spurious 'Stock Entry Type's from 'consumed' calculation * fix: filter using purpose, make requested changes Co-authored-by: Ankush Menat --- .../itemwise_recommended_reorder_level.py | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/erpnext/stock/report/itemwise_recommended_reorder_level/itemwise_recommended_reorder_level.py b/erpnext/stock/report/itemwise_recommended_reorder_level/itemwise_recommended_reorder_level.py index 5df3fa8067..2f70523264 100644 --- a/erpnext/stock/report/itemwise_recommended_reorder_level/itemwise_recommended_reorder_level.py +++ b/erpnext/stock/report/itemwise_recommended_reorder_level/itemwise_recommended_reorder_level.py @@ -55,19 +55,31 @@ def get_item_info(filters): def get_consumed_items(condition): + purpose_to_exclude = [ + "Material Transfer for Manufacture", + "Material Transfer", + "Send to Subcontractor" + ] + + condition += """ + and ( + purpose is NULL + or purpose not in ({}) + ) + """.format(', '.join([f"'{p}'" for p in purpose_to_exclude])) + condition = condition.replace("posting_date", "sle.posting_date") + consumed_items = frappe.db.sql(""" select item_code, abs(sum(actual_qty)) as consumed_qty - from `tabStock Ledger Entry` - where actual_qty < 0 + from `tabStock Ledger Entry` as sle left join `tabStock Entry` as se + on sle.voucher_no = se.name + where + actual_qty < 0 and voucher_type not in ('Delivery Note', 'Sales Invoice') %s - group by item_code - """ % condition, as_dict=1) - - consumed_items_map = {} - for item in consumed_items: - consumed_items_map.setdefault(item.item_code, item.consumed_qty) + group by item_code""" % condition, as_dict=1) + consumed_items_map = {item.item_code : item.consumed_qty for item in consumed_items} return consumed_items_map def get_delivered_items(condition): From 119b27b97f8cd2092c8e915e5283dbd26876d1ce Mon Sep 17 00:00:00 2001 From: Jannat Patel <31363128+pateljannat@users.noreply.github.com> Date: Mon, 19 Apr 2021 12:46:14 +0530 Subject: [PATCH 29/31] feat: Delayed Tasks Summary (#25024) * feat: delayed deliverables summary * fix: sider * fix: renamed to delayed tasks * fix: renamed test * fix: test * fix: sider * fix: dates, validations and chart * fix: space and column width * feat: Sort tasks by descending order of delay Co-authored-by: Rucha Mahabal --- erpnext/projects/doctype/task/task.json | 15 +- erpnext/projects/doctype/task/task.py | 5 + .../report/delayed_tasks_summary/__init__.py | 0 .../delayed_tasks_summary.js | 41 ++++++ .../delayed_tasks_summary.json | 29 ++++ .../delayed_tasks_summary.py | 133 ++++++++++++++++++ .../test_delayed_tasks_summary.py | 54 +++++++ .../projects/workspace/projects/projects.json | 13 +- 8 files changed, 286 insertions(+), 4 deletions(-) create mode 100644 erpnext/projects/report/delayed_tasks_summary/__init__.py create mode 100644 erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.js create mode 100644 erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.json create mode 100644 erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.py create mode 100644 erpnext/projects/report/delayed_tasks_summary/test_delayed_tasks_summary.py diff --git a/erpnext/projects/doctype/task/task.json b/erpnext/projects/doctype/task/task.json index 160cc5812f..ef4740d9ee 100644 --- a/erpnext/projects/doctype/task/task.json +++ b/erpnext/projects/doctype/task/task.json @@ -11,15 +11,16 @@ "project", "issue", "type", + "color", "is_group", "is_template", "column_break0", "status", "priority", "task_weight", - "completed_by", - "color", "parent_task", + "completed_by", + "completed_on", "sb_timeline", "exp_start_date", "expected_time", @@ -358,6 +359,7 @@ "read_only": 1 }, { + "depends_on": "eval: doc.status == \"Completed\"", "fieldname": "completed_by", "fieldtype": "Link", "label": "Completed By", @@ -381,6 +383,13 @@ "fieldname": "duration", "fieldtype": "Int", "label": "Duration (Days)" + }, + { + "depends_on": "eval: doc.status == \"Completed\"", + "fieldname": "completed_on", + "fieldtype": "Date", + "label": "Completed On", + "mandatory_depends_on": "eval: doc.status == \"Completed\"" } ], "icon": "fa fa-check", @@ -388,7 +397,7 @@ "is_tree": 1, "links": [], "max_attachments": 5, - "modified": "2020-12-28 11:32:58.714991", + "modified": "2021-04-16 12:46:51.556741", "modified_by": "Administrator", "module": "Projects", "name": "Task", diff --git a/erpnext/projects/doctype/task/task.py b/erpnext/projects/doctype/task/task.py index 855ff5f83e..d1583f1473 100755 --- a/erpnext/projects/doctype/task/task.py +++ b/erpnext/projects/doctype/task/task.py @@ -36,6 +36,7 @@ class Task(NestedSet): self.validate_status() self.update_depends_on() self.validate_dependencies_for_template_task() + self.validate_completed_on() def validate_dates(self): if self.exp_start_date and self.exp_end_date and getdate(self.exp_start_date) > getdate(self.exp_end_date): @@ -100,6 +101,10 @@ class Task(NestedSet): dependent_task_format = """{0}""".format(task.task) frappe.throw(_("Dependent Task {0} is not a Template Task").format(dependent_task_format)) + def validate_completed_on(self): + if self.completed_on and getdate(self.completed_on) > getdate(): + frappe.throw(_("Completed On cannot be greater than Today")) + def update_depends_on(self): depends_on_tasks = self.depends_on_tasks or "" for d in self.depends_on: diff --git a/erpnext/projects/report/delayed_tasks_summary/__init__.py b/erpnext/projects/report/delayed_tasks_summary/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.js b/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.js new file mode 100644 index 0000000000..5aa44c0a8c --- /dev/null +++ b/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.js @@ -0,0 +1,41 @@ +// Copyright (c) 2016, Frappe Technologies Pvt. Ltd. and contributors +// For license information, please see license.txt +/* eslint-disable */ + +frappe.query_reports["Delayed Tasks Summary"] = { + "filters": [ + { + "fieldname": "from_date", + "label": __("From Date"), + "fieldtype": "Date" + }, + { + "fieldname": "to_date", + "label": __("To Date"), + "fieldtype": "Date" + }, + { + "fieldname": "priority", + "label": __("Priority"), + "fieldtype": "Select", + "options": ["", "Low", "Medium", "High", "Urgent"] + }, + { + "fieldname": "status", + "label": __("Status"), + "fieldtype": "Select", + "options": ["", "Open", "Working","Pending Review","Overdue","Completed"] + }, + ], + "formatter": function(value, row, column, data, default_formatter) { + value = default_formatter(value, row, column, data); + if (column.id == "delay") { + if (data["delay"] > 0) { + value = `

${value}

`; + } else { + value = `

${value}

`; + } + } + return value + } +}; diff --git a/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.json b/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.json new file mode 100644 index 0000000000..100c422433 --- /dev/null +++ b/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.json @@ -0,0 +1,29 @@ +{ + "add_total_row": 0, + "columns": [], + "creation": "2021-03-25 15:03:19.857418", + "disable_prepared_report": 0, + "disabled": 0, + "docstatus": 0, + "doctype": "Report", + "filters": [], + "idx": 0, + "is_standard": "Yes", + "modified": "2021-04-15 15:49:35.432486", + "modified_by": "Administrator", + "module": "Projects", + "name": "Delayed Tasks Summary", + "owner": "Administrator", + "prepared_report": 0, + "ref_doctype": "Task", + "report_name": "Delayed Tasks Summary", + "report_type": "Script Report", + "roles": [ + { + "role": "Projects User" + }, + { + "role": "Projects Manager" + } + ] +} \ No newline at end of file diff --git a/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.py b/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.py new file mode 100644 index 0000000000..cdabe6487e --- /dev/null +++ b/erpnext/projects/report/delayed_tasks_summary/delayed_tasks_summary.py @@ -0,0 +1,133 @@ +# Copyright (c) 2013, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +from __future__ import unicode_literals +import frappe +from frappe.utils import date_diff, nowdate + +def execute(filters=None): + columns, data = [], [] + data = get_data(filters) + columns = get_columns() + charts = get_chart_data(data) + return columns, data, None, charts + +def get_data(filters): + conditions = get_conditions(filters) + tasks = frappe.get_all("Task", + filters = conditions, + fields = ["name", "subject", "exp_start_date", "exp_end_date", + "status", "priority", "completed_on", "progress"], + order_by="creation" + ) + for task in tasks: + if task.exp_end_date: + if task.completed_on: + task.delay = date_diff(task.completed_on, task.exp_end_date) + elif task.status == "Completed": + # task is completed but completed on is not set (for older tasks) + task.delay = 0 + else: + # task not completed + task.delay = date_diff(nowdate(), task.exp_end_date) + else: + # task has no end date, hence no delay + task.delay = 0 + + # Sort by descending order of delay + tasks.sort(key=lambda x: x["delay"], reverse=True) + return tasks + +def get_conditions(filters): + conditions = frappe._dict() + keys = ["priority", "status"] + for key in keys: + if filters.get(key): + conditions[key] = filters.get(key) + if filters.get("from_date"): + conditions.exp_end_date = [">=", filters.get("from_date")] + if filters.get("to_date"): + conditions.exp_start_date = ["<=", filters.get("to_date")] + return conditions + +def get_chart_data(data): + delay, on_track = 0, 0 + for entry in data: + if entry.get("delay") > 0: + delay = delay + 1 + else: + on_track = on_track + 1 + charts = { + "data": { + "labels": ["On Track", "Delayed"], + "datasets": [ + { + "name": "Delayed", + "values": [on_track, delay] + } + ] + }, + "type": "percentage", + "colors": ["#84D5BA", "#CB4B5F"] + } + return charts + +def get_columns(): + columns = [ + { + "fieldname": "name", + "fieldtype": "Link", + "label": "Task", + "options": "Task", + "width": 150 + }, + { + "fieldname": "subject", + "fieldtype": "Data", + "label": "Subject", + "width": 200 + }, + { + "fieldname": "status", + "fieldtype": "Data", + "label": "Status", + "width": 100 + }, + { + "fieldname": "priority", + "fieldtype": "Data", + "label": "Priority", + "width": 80 + }, + { + "fieldname": "progress", + "fieldtype": "Data", + "label": "Progress (%)", + "width": 120 + }, + { + "fieldname": "exp_start_date", + "fieldtype": "Date", + "label": "Expected Start Date", + "width": 150 + }, + { + "fieldname": "exp_end_date", + "fieldtype": "Date", + "label": "Expected End Date", + "width": 150 + }, + { + "fieldname": "completed_on", + "fieldtype": "Date", + "label": "Actual End Date", + "width": 130 + }, + { + "fieldname": "delay", + "fieldtype": "Data", + "label": "Delay (In Days)", + "width": 120 + } + ] + return columns diff --git a/erpnext/projects/report/delayed_tasks_summary/test_delayed_tasks_summary.py b/erpnext/projects/report/delayed_tasks_summary/test_delayed_tasks_summary.py new file mode 100644 index 0000000000..dbeedb4be9 --- /dev/null +++ b/erpnext/projects/report/delayed_tasks_summary/test_delayed_tasks_summary.py @@ -0,0 +1,54 @@ +from __future__ import unicode_literals +import unittest +import frappe +from frappe.utils import nowdate, add_days, add_months +from erpnext.projects.doctype.task.test_task import create_task +from erpnext.projects.report.delayed_tasks_summary.delayed_tasks_summary import execute + +class TestDelayedTasksSummary(unittest.TestCase): + @classmethod + def setUp(self): + task1 = create_task("_Test Task 98", add_days(nowdate(), -10), nowdate()) + create_task("_Test Task 99", add_days(nowdate(), -10), add_days(nowdate(), -1)) + + task1.status = "Completed" + task1.completed_on = add_days(nowdate(), -1) + task1.save() + + def test_delayed_tasks_summary(self): + filters = frappe._dict({ + "from_date": add_months(nowdate(), -1), + "to_date": nowdate(), + "priority": "Low", + "status": "Open" + }) + expected_data = [ + { + "subject": "_Test Task 99", + "status": "Open", + "priority": "Low", + "delay": 1 + }, + { + "subject": "_Test Task 98", + "status": "Completed", + "priority": "Low", + "delay": -1 + } + ] + report = execute(filters) + data = list(filter(lambda x: x.subject == "_Test Task 99", report[1]))[0] + + for key in ["subject", "status", "priority", "delay"]: + self.assertEqual(expected_data[0].get(key), data.get(key)) + + filters.status = "Completed" + report = execute(filters) + data = list(filter(lambda x: x.subject == "_Test Task 98", report[1]))[0] + + for key in ["subject", "status", "priority", "delay"]: + self.assertEqual(expected_data[1].get(key), data.get(key)) + + def tearDown(self): + for task in ["_Test Task 98", "_Test Task 99"]: + frappe.get_doc("Task", {"subject": task}).delete() \ No newline at end of file diff --git a/erpnext/projects/workspace/projects/projects.json b/erpnext/projects/workspace/projects/projects.json index dbbd7e1458..0ec17029a2 100644 --- a/erpnext/projects/workspace/projects/projects.json +++ b/erpnext/projects/workspace/projects/projects.json @@ -15,6 +15,7 @@ "hide_custom": 0, "icon": "project", "idx": 0, + "is_default": 0, "is_standard": 1, "label": "Projects", "links": [ @@ -148,9 +149,19 @@ "link_type": "Report", "onboard": 0, "type": "Link" + }, + { + "dependencies": "Task", + "hidden": 0, + "is_query_report": 1, + "label": "Delayed Tasks Summary", + "link_to": "Delayed Tasks Summary", + "link_type": "Report", + "onboard": 0, + "type": "Link" } ], - "modified": "2020-12-01 13:38:37.856224", + "modified": "2021-03-26 16:32:00.628561", "modified_by": "Administrator", "module": "Projects", "name": "Projects", From ceba5774be6910d6c48f0f69e3f4adc1345b1b4b Mon Sep 17 00:00:00 2001 From: Rakshith N <36509967+rakshithrddy@users.noreply.github.com> Date: Mon, 19 Apr 2021 13:21:49 +0530 Subject: [PATCH 30/31] fix(pos): special character scanning in point of sale (#25353) Co-authored-by: rakshith.n Co-authored-by: Saqib --- .../page/point_of_sale/pos_item_selector.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/erpnext/selling/page/point_of_sale/pos_item_selector.js b/erpnext/selling/page/point_of_sale/pos_item_selector.js index e0d5b73166..9fb3943b53 100644 --- a/erpnext/selling/page/point_of_sale/pos_item_selector.js +++ b/erpnext/selling/page/point_of_sale/pos_item_selector.js @@ -159,6 +159,31 @@ erpnext.PointOfSale.ItemSelector = class { bind_events() { const me = this; window.onScan = onScan; + + onScan.decodeKeyEvent = function (oEvent) { + var iCode = this._getNormalizedKeyNum(oEvent); + switch (true) { + case iCode >= 48 && iCode <= 90: // numbers and letters + case iCode >= 106 && iCode <= 111: // operations on numeric keypad (+, -, etc.) + case (iCode >= 160 && iCode <= 164) || iCode == 170: // ^ ! # $ * + case iCode >= 186 && iCode <= 194: // (; = , - . / `) + case iCode >= 219 && iCode <= 222: // ([ \ ] ') + if (oEvent.key !== undefined && oEvent.key !== '') { + return oEvent.key; + } + + var sDecoded = String.fromCharCode(iCode); + switch (oEvent.shiftKey) { + case false: sDecoded = sDecoded.toLowerCase(); break; + case true: sDecoded = sDecoded.toUpperCase(); break; + } + return sDecoded; + case iCode >= 96 && iCode <= 105: // numbers on numeric keypad + return 0 + (iCode - 96); + } + return ''; + }; + onScan.attachTo(document, { onScan: (sScancode) => { if (this.search_field && this.$component.is(':visible')) { From cb718fce88e3dd7866980333237cec25b9a72acf Mon Sep 17 00:00:00 2001 From: Deepesh Garg <42651287+deepeshgarg007@users.noreply.github.com> Date: Mon, 19 Apr 2021 13:25:15 +0530 Subject: [PATCH 31/31] feat: Role to allow over billing, delivery, receipt (#24854) * feat: Role to allow over billing, delivery, receipt * fix: Typo --- .../doctype/accounts_settings/accounts_settings.json | 10 +++++++++- erpnext/controllers/accounts_controller.py | 4 +++- erpnext/controllers/status_updater.py | 10 +++++++--- .../stock/doctype/stock_settings/stock_settings.json | 10 +++++++++- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index a3c29b6d64..e1276e7da3 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -12,6 +12,7 @@ "frozen_accounts_modifier", "determine_address_tax_category_from", "over_billing_allowance", + "role_allowed_to_over_bill", "column_break_4", "credit_controller", "check_supplier_invoice_uniqueness", @@ -226,6 +227,13 @@ "fieldname": "delete_linked_ledger_entries", "fieldtype": "Check", "label": "Delete Accounting and Stock Ledger Entries on deletion of Transaction" + }, + { + "description": "Users with this role are allowed to over bill above the allowance percentage", + "fieldname": "role_allowed_to_over_bill", + "fieldtype": "Link", + "label": "Role Allowed to Over Bill ", + "options": "Role" } ], "icon": "icon-cog", @@ -233,7 +241,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2021-01-05 13:04:00.118892", + "modified": "2021-03-11 18:52:05.601996", "modified_by": "Administrator", "module": "Accounts", "name": "Accounts Settings", diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 33fbf1c0b9..d36e7b03f4 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -717,7 +717,9 @@ class AccountsController(TransactionBase): total_billed_amt = abs(total_billed_amt) max_allowed_amt = abs(max_allowed_amt) - if total_billed_amt - max_allowed_amt > 0.01: + role_allowed_to_over_bill = frappe.db.get_single_value('Accounts Settings', 'role_allowed_to_over_bill') + + if total_billed_amt - max_allowed_amt > 0.01 and role_allowed_to_over_bill not in frappe.get_roles(): frappe.throw(_("Cannot overbill for Item {0} in row {1} more than {2}. To allow over-billing, please set allowance in Accounts Settings") .format(item.item_code, item.idx, max_allowed_amt)) diff --git a/erpnext/controllers/status_updater.py b/erpnext/controllers/status_updater.py index cdb6d244a6..5276da9720 100644 --- a/erpnext/controllers/status_updater.py +++ b/erpnext/controllers/status_updater.py @@ -201,10 +201,14 @@ class StatusUpdater(Document): get_allowance_for(item['item_code'], self.item_allowance, self.global_qty_allowance, self.global_amount_allowance, qty_or_amount) - overflow_percent = ((item[args['target_field']] - item[args['target_ref_field']]) / - item[args['target_ref_field']]) * 100 + role_allowed_to_over_deliver_receive = frappe.db.get_single_value('Stock Settings', 'role_allowed_to_over_deliver_receive') + role_allowed_to_over_bill = frappe.db.get_single_value('Accounts Settings', 'role_allowed_to_over_bill') + role = role_allowed_to_over_deliver_receive if qty_or_amount == 'qty' else role_allowed_to_over_bill - if overflow_percent - allowance > 0.01: + overflow_percent = ((item[args['target_field']] - item[args['target_ref_field']]) / + item[args['target_ref_field']]) * 100 + + if overflow_percent - allowance > 0.01 and role not in frappe.get_roles(): item['max_allowed'] = flt(item[args['target_ref_field']] * (100+allowance)/100) item['reduce_by'] = item[args['target_field']] - item['max_allowed'] diff --git a/erpnext/stock/doctype/stock_settings/stock_settings.json b/erpnext/stock/doctype/stock_settings/stock_settings.json index 84af57b48d..f18eabc84b 100644 --- a/erpnext/stock/doctype/stock_settings/stock_settings.json +++ b/erpnext/stock/doctype/stock_settings/stock_settings.json @@ -13,6 +13,7 @@ "column_break_4", "valuation_method", "over_delivery_receipt_allowance", + "role_allowed_to_over_deliver_receive", "action_if_quality_inspection_is_not_submitted", "show_barcode_field", "clean_description_html", @@ -234,6 +235,13 @@ "fieldname": "disable_serial_no_and_batch_selector", "fieldtype": "Check", "label": "Disable Serial No And Batch Selector" + }, + { + "description": "Users with this role are allowed to over deliver/receive against orders above the allowance percentage", + "fieldname": "role_allowed_to_over_deliver_receive", + "fieldtype": "Link", + "label": "Role Allowed to Over Deliver/Receive", + "options": "Role" } ], "icon": "icon-cog", @@ -241,7 +249,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2021-01-18 13:15:38.352796", + "modified": "2021-03-11 18:48:14.513055", "modified_by": "Administrator", "module": "Stock", "name": "Stock Settings",