From 3764c01028fafc72abd8686335bac8ad4037e136 Mon Sep 17 00:00:00 2001 From: RitvikSardana <65544983+RitvikSardana@users.noreply.github.com> Date: Mon, 18 Sep 2023 13:07:14 +0530 Subject: [PATCH] fix: accounting dimensions required while creating POS Profile (#36668) * fix: accounting dimensions required while creating POS Porfile * fix: accounting dimensions fetched while closing POS * chore: code cleanup * fix: ad set in final consolidated invoice * chore: code cleanup * chore: code cleanup * fix: accounting dimension validation from backend * chore: code cleanup * chore: code cleanup * chore: code cleanup * fix: added edge case when acc dimension is created after creating pos profile * test: added test case for Pos Closing For Required Accounting Dimension In Pos Profile * test: fixed the test case * test: fixing test case * fix: changed test case location * test: fixing test case * test: fixing test case --------- Co-authored-by: Ritvik Sardana --- .../accounting_dimension.py | 27 ++++++------ .../test_accounting_dimension.py | 14 +++++- .../test_pos_closing_entry.py | 44 +++++++++++++++++++ .../pos_invoice_merge_log.py | 23 ++++++++-- .../doctype/pos_profile/pos_profile.js | 2 +- .../doctype/pos_profile/pos_profile.py | 35 ++++++++++++++- .../doctype/pos_profile/test_pos_profile.py | 9 +++- 7 files changed, 132 insertions(+), 22 deletions(-) diff --git a/erpnext/accounts/doctype/accounting_dimension/accounting_dimension.py b/erpnext/accounts/doctype/accounting_dimension/accounting_dimension.py index cfe5e6e8009..3a2c3cbeeb1 100644 --- a/erpnext/accounts/doctype/accounting_dimension/accounting_dimension.py +++ b/erpnext/accounts/doctype/accounting_dimension/accounting_dimension.py @@ -265,20 +265,21 @@ def get_dimension_with_children(doctype, dimensions): @frappe.whitelist() def get_dimensions(with_cost_center_and_project=False): - dimension_filters = frappe.db.sql( - """ - SELECT label, fieldname, document_type - FROM `tabAccounting Dimension` - WHERE disabled = 0 - """, - as_dict=1, - ) - default_dimensions = frappe.db.sql( - """SELECT p.fieldname, c.company, c.default_dimension - FROM `tabAccounting Dimension Detail` c, `tabAccounting Dimension` p - WHERE c.parent = p.name""", - as_dict=1, + c = frappe.qb.DocType("Accounting Dimension Detail") + p = frappe.qb.DocType("Accounting Dimension") + dimension_filters = ( + frappe.qb.from_(p) + .select(p.label, p.fieldname, p.document_type) + .where(p.disabled == 0) + .run(as_dict=1) + ) + default_dimensions = ( + frappe.qb.from_(c) + .inner_join(p) + .on(c.parent == p.name) + .select(p.fieldname, c.company, c.default_dimension) + .run(as_dict=1) ) if isinstance(with_cost_center_and_project, str): diff --git a/erpnext/accounts/doctype/accounting_dimension/test_accounting_dimension.py b/erpnext/accounts/doctype/accounting_dimension/test_accounting_dimension.py index 25ef2ea5c2c..cb7f5f5da78 100644 --- a/erpnext/accounts/doctype/accounting_dimension/test_accounting_dimension.py +++ b/erpnext/accounts/doctype/accounting_dimension/test_accounting_dimension.py @@ -84,12 +84,22 @@ def create_dimension(): frappe.set_user("Administrator") if not frappe.db.exists("Accounting Dimension", {"document_type": "Department"}): - frappe.get_doc( + dimension = frappe.get_doc( { "doctype": "Accounting Dimension", "document_type": "Department", } - ).insert() + ) + dimension.append( + "dimension_defaults", + { + "company": "_Test Company", + "reference_document": "Department", + "default_dimension": "_Test Department - _TC", + }, + ) + dimension.insert() + dimension.save() else: dimension = frappe.get_doc("Accounting Dimension", "Department") dimension.disabled = 0 diff --git a/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py b/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py index 93ba90ad9f9..62b342a3d20 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py +++ b/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py @@ -5,6 +5,10 @@ import unittest import frappe +from erpnext.accounts.doctype.accounting_dimension.test_accounting_dimension import ( + create_dimension, + disable_dimension, +) from erpnext.accounts.doctype.pos_closing_entry.pos_closing_entry import ( make_closing_entry_from_opening, ) @@ -140,6 +144,43 @@ class TestPOSClosingEntry(unittest.TestCase): pos_inv1.load_from_db() self.assertEqual(pos_inv1.status, "Paid") + def test_pos_closing_for_required_accounting_dimension_in_pos_profile(self): + """ + test case to check whether we can create POS Closing Entry without mandatory accounting dimension + """ + + create_dimension() + pos_profile = make_pos_profile(do_not_insert=1, do_not_set_accounting_dimension=1) + + self.assertRaises(frappe.ValidationError, pos_profile.insert) + + pos_profile.location = "Block 1" + pos_profile.insert() + self.assertTrue(frappe.db.exists("POS Profile", pos_profile.name)) + + test_user = init_user_and_profile(do_not_create_pos_profile=1) + + opening_entry = create_opening_entry(pos_profile, test_user.name) + pos_inv1 = create_pos_invoice(rate=350, do_not_submit=1, pos_profile=pos_profile.name) + pos_inv1.append("payments", {"mode_of_payment": "Cash", "account": "Cash - _TC", "amount": 3500}) + pos_inv1.submit() + + # if in between a mandatory accounting dimension is added to the POS Profile then + accounting_dimension_department = frappe.get_doc("Accounting Dimension", {"name": "Department"}) + accounting_dimension_department.dimension_defaults[0].mandatory_for_bs = 1 + accounting_dimension_department.save() + + pcv_doc = make_closing_entry_from_opening(opening_entry) + # will assert coz the new mandatory accounting dimension bank is not set in POS Profile + self.assertRaises(frappe.ValidationError, pcv_doc.submit) + + accounting_dimension_department = frappe.get_doc( + "Accounting Dimension Detail", {"parent": "Department"} + ) + accounting_dimension_department.mandatory_for_bs = 0 + accounting_dimension_department.save() + disable_dimension() + def init_user_and_profile(**args): user = "test@example.com" @@ -149,6 +190,9 @@ def init_user_and_profile(**args): test_user.add_roles(*roles) frappe.set_user(user) + if args.get("do_not_create_pos_profile"): + return test_user + pos_profile = make_pos_profile(**args) pos_profile.append("applicable_for_users", {"default": 1, "user": user}) diff --git a/erpnext/accounts/doctype/pos_invoice_merge_log/pos_invoice_merge_log.py b/erpnext/accounts/doctype/pos_invoice_merge_log/pos_invoice_merge_log.py index b587ce603f4..d42b1e4cd1d 100644 --- a/erpnext/accounts/doctype/pos_invoice_merge_log/pos_invoice_merge_log.py +++ b/erpnext/accounts/doctype/pos_invoice_merge_log/pos_invoice_merge_log.py @@ -12,6 +12,8 @@ from frappe.utils import cint, flt, get_time, getdate, nowdate, nowtime from frappe.utils.background_jobs import enqueue, is_job_enqueued from frappe.utils.scheduler import is_scheduler_inactive +from erpnext.accounts.doctype.pos_profile.pos_profile import required_accounting_dimensions + class POSInvoiceMergeLog(Document): def validate(self): @@ -163,7 +165,8 @@ class POSInvoiceMergeLog(Document): for i in items: if ( i.item_code == item.item_code - and not i.serial_and_batch_bundle + and not i.serial_no + and not i.batch_no and i.uom == item.uom and i.net_rate == item.net_rate and i.warehouse == item.warehouse @@ -238,6 +241,22 @@ class POSInvoiceMergeLog(Document): invoice.disable_rounded_total = cint( frappe.db.get_value("POS Profile", invoice.pos_profile, "disable_rounded_total") ) + accounting_dimensions = required_accounting_dimensions() + dimension_values = frappe.db.get_value( + "POS Profile", {"name": invoice.pos_profile}, accounting_dimensions, as_dict=1 + ) + for dimension in accounting_dimensions: + dimension_value = dimension_values.get(dimension) + + if not dimension_value: + frappe.throw( + _("Please set Accounting Dimension {} in {}").format( + frappe.bold(frappe.unscrub(dimension)), + frappe.get_desk_link("POS Profile", invoice.pos_profile), + ) + ) + + invoice.set(dimension, dimension_value) if self.merge_invoices_based_on == "Customer Group": invoice.flags.ignore_pos_profile = True @@ -424,11 +443,9 @@ def create_merge_logs(invoice_by_customer, closing_entry=None): ) merge_log.customer = customer merge_log.pos_closing_entry = closing_entry.get("name") if closing_entry else None - merge_log.set("pos_invoices", _invoices) merge_log.save(ignore_permissions=True) merge_log.submit() - if closing_entry: closing_entry.set_status(update=True, status="Submitted") closing_entry.db_set("error_message", "") diff --git a/erpnext/accounts/doctype/pos_profile/pos_profile.js b/erpnext/accounts/doctype/pos_profile/pos_profile.js index 0a89aee8e9c..ceaafaa3b12 100755 --- a/erpnext/accounts/doctype/pos_profile/pos_profile.js +++ b/erpnext/accounts/doctype/pos_profile/pos_profile.js @@ -1,6 +1,5 @@ // Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors // License: GNU General Public License v3. See license.txt - frappe.ui.form.on('POS Profile', { setup: function(frm) { frm.set_query("selling_price_list", function() { @@ -140,6 +139,7 @@ frappe.ui.form.on('POS Profile', { company: function(frm) { frm.trigger("toggle_display_account_head"); erpnext.accounts.dimensions.update_dimension(frm, frm.doctype); + }, toggle_display_account_head: function(frm) { diff --git a/erpnext/accounts/doctype/pos_profile/pos_profile.py b/erpnext/accounts/doctype/pos_profile/pos_profile.py index e8aee737f29..58be2d3e5c0 100644 --- a/erpnext/accounts/doctype/pos_profile/pos_profile.py +++ b/erpnext/accounts/doctype/pos_profile/pos_profile.py @@ -3,7 +3,7 @@ import frappe -from frappe import _, msgprint +from frappe import _, msgprint, scrub, unscrub from frappe.model.document import Document from frappe.utils import get_link_to_form, now @@ -14,6 +14,21 @@ class POSProfile(Document): self.validate_all_link_fields() self.validate_duplicate_groups() self.validate_payment_methods() + self.validate_accounting_dimensions() + + def validate_accounting_dimensions(self): + acc_dim_names = required_accounting_dimensions() + for acc_dim in acc_dim_names: + if not self.get(acc_dim): + frappe.throw( + _( + "{0} is a mandatory Accounting Dimension.
" + "Please set a value for {0} in Accounting Dimensions section." + ).format( + unscrub(frappe.bold(acc_dim)), + ), + title=_("Mandatory Accounting Dimension"), + ) def validate_default_profile(self): for row in self.applicable_for_users: @@ -152,6 +167,24 @@ def get_child_nodes(group_type, root): ) +def required_accounting_dimensions(): + + p = frappe.qb.DocType("Accounting Dimension") + c = frappe.qb.DocType("Accounting Dimension Detail") + + acc_dim_doc = ( + frappe.qb.from_(p) + .inner_join(c) + .on(p.name == c.parent) + .select(c.parent) + .where((c.mandatory_for_bs == 1) | (c.mandatory_for_pl == 1)) + .where(p.disabled == 0) + ).run(as_dict=1) + + acc_dim_names = [scrub(d.parent) for d in acc_dim_doc] + return acc_dim_names + + @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def pos_profile_query(doctype, txt, searchfield, start, page_len, filters): diff --git a/erpnext/accounts/doctype/pos_profile/test_pos_profile.py b/erpnext/accounts/doctype/pos_profile/test_pos_profile.py index 788aa62701d..b468ad3fe9b 100644 --- a/erpnext/accounts/doctype/pos_profile/test_pos_profile.py +++ b/erpnext/accounts/doctype/pos_profile/test_pos_profile.py @@ -5,7 +5,10 @@ import unittest import frappe -from erpnext.accounts.doctype.pos_profile.pos_profile import get_child_nodes +from erpnext.accounts.doctype.pos_profile.pos_profile import ( + get_child_nodes, + required_accounting_dimensions, +) from erpnext.stock.get_item_details import get_pos_profile test_dependencies = ["Item"] @@ -118,6 +121,7 @@ def make_pos_profile(**args): "warehouse": args.warehouse or "_Test Warehouse - _TC", "write_off_account": args.write_off_account or "_Test Write Off - _TC", "write_off_cost_center": args.write_off_cost_center or "_Test Write Off Cost Center - _TC", + "location": "Block 1" if not args.do_not_set_accounting_dimension else None, } ) @@ -132,6 +136,7 @@ def make_pos_profile(**args): pos_profile.append("payments", {"mode_of_payment": "Cash", "default": 1}) if not frappe.db.exists("POS Profile", args.name or "_Test POS Profile"): - pos_profile.insert() + if not args.get("do_not_insert"): + pos_profile.insert() return pos_profile