From cd3976f7d24a703c80858237e9f51569b9323bea Mon Sep 17 00:00:00 2001 From: Saqib Date: Mon, 25 Nov 2019 12:24:34 +0530 Subject: [PATCH] Asset cancellation fix (#19671) * fix: remove asset movement mandatory fields * fix: label for reference doctype --- erpnext/assets/doctype/asset/asset.py | 25 +++------ .../doctype/asset_movement/asset_movement.js | 51 +++---------------- .../asset_movement/asset_movement.json | 15 +++--- .../doctype/asset_movement/asset_movement.py | 2 +- erpnext/controllers/buying_controller.py | 13 +++-- 5 files changed, 32 insertions(+), 74 deletions(-) diff --git a/erpnext/assets/doctype/asset/asset.py b/erpnext/assets/doctype/asset/asset.py index 546f374094..56341ed1b1 100644 --- a/erpnext/assets/doctype/asset/asset.py +++ b/erpnext/assets/doctype/asset/asset.py @@ -125,12 +125,14 @@ class Asset(AccountsController): frappe.throw(_("Available-for-use Date should be after purchase date")) def cancel_auto_gen_movement(self): - reference_docname = self.purchase_invoice or self.purchase_receipt - movement = frappe.db.get_all('Asset Movement', filters={ 'reference_name': reference_docname, 'docstatus': 1 }) - if len(movement) > 1: + movements = frappe.db.sql( + """SELECT asm.name, asm.docstatus + FROM `tabAsset Movement` asm, `tabAsset Movement Item` asm_item + WHERE asm_item.parent=asm.name and asm_item.asset=%s and asm.docstatus=1""", self.name, as_dict=1) + if len(movements) > 1: frappe.throw(_('Asset has multiple Asset Movement Entries which has to be \ cancelled manually to cancel this asset.')) - movement = frappe.get_doc('Asset Movement', movement[0].get('name')) + movement = frappe.get_doc('Asset Movement', movements[0].get('name')) movement.flags.ignore_validate = True movement.cancel() @@ -658,23 +660,10 @@ def make_asset_movement(assets, purpose=None): frappe.throw(_('Atleast one asset has to be selected.')) asset_movement = frappe.new_doc("Asset Movement") - asset_movement.purpose = purpose - prev_reference_docname = '' - + asset_movement.quantity = len(assets) for asset in assets: asset = frappe.get_doc('Asset', asset.get('name')) - # get PR/PI linked with asset - reference_docname = asset.get('purchase_receipt') if asset.get('purchase_receipt') \ - else asset.get('purchase_invoice') - # checks if all the assets are linked with a single PR/PI - if prev_reference_docname == '': - prev_reference_docname = reference_docname - elif prev_reference_docname != reference_docname: - frappe.throw(_('Assets selected should belong to same reference document.')) - asset_movement.company = asset.get('company') - asset_movement.reference_doctype = 'Purchase Receipt' if asset.get('purchase_receipt') else 'Purchase Invoice' - asset_movement.reference_name = prev_reference_docname asset_movement.append("assets", { 'asset': asset.get('name'), 'source_location': asset.get('location'), diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.js b/erpnext/assets/doctype/asset_movement/asset_movement.js index 89977e2952..06d8879091 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.js +++ b/erpnext/assets/doctype/asset_movement/asset_movement.js @@ -31,6 +31,13 @@ frappe.ui.form.on('Asset Movement', { name: ["in", ["Purchase Receipt", "Purchase Invoice"]] } }; + }), + frm.set_query("asset", "assets", () => { + return { + filters: { + status: ["not in", ["Draft"]] + } + } }) }, @@ -76,50 +83,6 @@ frappe.ui.form.on('Asset Movement', { }); }); frm.refresh_field('assets'); - }, - - reference_name: function(frm) { - if (frm.doc.reference_name && frm.doc.reference_doctype) { - const reference_doctype = frm.doc.reference_doctype === 'Purchase Invoice' ? 'purchase_invoice' : 'purchase_receipt'; - // On selection of reference name, - // sets query to display assets linked to that reference doc - frm.set_query('asset', 'assets', function() { - return { - filters: { - [reference_doctype] : frm.doc.reference_name - } - }; - }); - - // fetches linked asset & adds to the assets table - frappe.db.get_list('Asset', { - fields: ['name', 'location', 'custodian'], - filters: { - [reference_doctype] : frm.doc.reference_name - } - }).then((docs) => { - if (docs.length == 0) { - frappe.msgprint(frappe._(`Please select ${frm.doc.reference_doctype} which has assets.`)); - frm.doc.reference_name = ''; - frm.refresh_field('reference_name'); - return; - } - frm.doc.assets = []; - docs.forEach(doc => { - frm.add_child('assets', { - asset: doc.name, - source_location: doc.location, - from_employee: doc.custodian - }); - frm.refresh_field('assets'); - }) - }).catch((err) => { - console.log(err); // eslint-disable-line - }); - } else { - // if reference is deleted then remove query - frm.set_query('asset', 'assets', () => ({ filters: {} })); - } } }); diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.json b/erpnext/assets/doctype/asset_movement/asset_movement.json index e62d684411..3472ab5d7d 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.json +++ b/erpnext/assets/doctype/asset_movement/asset_movement.json @@ -9,12 +9,12 @@ "purpose", "column_break_4", "transaction_date", + "section_break_10", + "assets", "reference", "reference_doctype", "column_break_9", "reference_name", - "section_break_10", - "assets", "amended_from" ], "fields": [ @@ -47,6 +47,7 @@ "fieldtype": "Column Break" }, { + "collapsible": 1, "fieldname": "reference", "fieldtype": "Section Break", "label": "Reference" @@ -54,18 +55,16 @@ { "fieldname": "reference_doctype", "fieldtype": "Link", - "label": "Reference Document", + "label": "Reference Document Type", "no_copy": 1, - "options": "DocType", - "reqd": 1 + "options": "DocType" }, { "fieldname": "reference_name", "fieldtype": "Dynamic Link", "label": "Reference Document Name", "no_copy": 1, - "options": "reference_doctype", - "reqd": 1 + "options": "reference_doctype" }, { "fieldname": "amended_from", @@ -93,7 +92,7 @@ } ], "is_submittable": 1, - "modified": "2019-11-21 14:35:51.880332", + "modified": "2019-11-23 13:28:47.256935", "modified_by": "Administrator", "module": "Assets", "name": "Asset Movement", diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.py b/erpnext/assets/doctype/asset_movement/asset_movement.py index 714845dfac..4e1822b2ce 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/asset_movement.py @@ -22,7 +22,7 @@ class AssetMovement(Document): if company != self.company: frappe.throw(_("Asset {0} does not belong to company {1}").format(d.asset, self.company)) - if not(d.source_location or d.target_location or d.from_employee or d.to_employee): + if not (d.source_location or d.target_location or d.from_employee or d.to_employee): frappe.throw(_("Either location or employee must be required")) def validate_location(self): diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index 3392850e96..d12643af82 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -641,7 +641,10 @@ class BuyingController(StockController): asset = frappe.get_doc('Asset', asset.name) if delete_asset and is_auto_create_enabled: # need to delete movements to delete assets otherwise throws link exists error - movements = frappe.db.get_all('Asset Movement', filters={ 'reference_name': self.name }) + movements = frappe.db.sql( + """SELECT asm.name + FROM `tabAsset Movement` asm, `tabAsset Movement Item` asm_item + WHERE asm_item.parent=asm.name and asm_item.asset=%s""", asset.name, as_dict=1) for movement in movements: frappe.delete_doc('Asset Movement', movement.name, force=1) frappe.delete_doc("Asset", asset.name, force=1) @@ -652,8 +655,12 @@ class BuyingController(StockController): asset.purchase_date = self.posting_date asset.supplier = self.supplier elif self.docstatus == 2: - asset.set(field, None) - asset.supplier = None + if asset.docstatus == 0: + asset.set(field, None) + asset.supplier = None + if asset.docstatus == 1 and delete_asset: + frappe.throw(_('Cannot cancel this document as it is linked with submitted asset {0}.\ + Please cancel the it to continue.').format(asset.name)) asset.flags.ignore_validate_update_after_submit = True asset.flags.ignore_mandatory = True