From 1f7eccdac5b9b71fe49f9524f3c1a0de1331c3f8 Mon Sep 17 00:00:00 2001 From: Khushi Rawat <142375893+khushi8112@users.noreply.github.com> Date: Wed, 4 Jun 2025 15:33:43 +0530 Subject: [PATCH 1/7] feat: added Transfer and Issue option in purpose (cherry picked from commit f5e5146021d0b8348142e010fdb7715f7f45d386) # Conflicts: # erpnext/assets/doctype/asset_movement/asset_movement.json --- .../doctype/asset_movement/asset_movement.json | 14 ++++++++++++-- .../doctype/asset_movement/asset_movement.py | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.json b/erpnext/assets/doctype/asset_movement/asset_movement.json index 5382f9e75f2..1bd1ad83ead 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.json +++ b/erpnext/assets/doctype/asset_movement/asset_movement.json @@ -33,7 +33,7 @@ "fieldname": "purpose", "fieldtype": "Select", "label": "Purpose", - "options": "\nIssue\nReceipt\nTransfer", + "options": "\nIssue\nReceipt\nTransfer\nTransfer and Issue", "reqd": 1 }, { @@ -93,10 +93,15 @@ "fieldtype": "Column Break" } ], + "grid_page_length": 50, "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], +<<<<<<< HEAD "modified": "2023-06-28 16:54:26.571083", +======= + "modified": "2025-05-30 17:01:55.864353", +>>>>>>> f5e5146021 (feat: added Transfer and Issue option in purpose) "modified_by": "Administrator", "module": "Assets", "name": "Asset Movement", @@ -149,7 +154,12 @@ "write": 1 } ], +<<<<<<< HEAD "sort_field": "modified", +======= + "row_format": "Dynamic", + "sort_field": "creation", +>>>>>>> f5e5146021 (feat: added Transfer and Issue option in purpose) "sort_order": "DESC", "states": [] -} \ No newline at end of file +} diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.py b/erpnext/assets/doctype/asset_movement/asset_movement.py index d51971c04e8..0c986d086ce 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/asset_movement.py @@ -24,7 +24,7 @@ class AssetMovement(Document): amended_from: DF.Link | None assets: DF.Table[AssetMovementItem] company: DF.Link - purpose: DF.Literal["", "Issue", "Receipt", "Transfer"] + purpose: DF.Literal["", "Issue", "Receipt", "Transfer", "Transfer and Issue"] reference_doctype: DF.Link | None reference_name: DF.DynamicLink | None transaction_date: DF.Datetime From 0c07dfadfe300befc5bfb0a9cffa1da1cef210c9 Mon Sep 17 00:00:00 2001 From: Khushi Rawat <142375893+khushi8112@users.noreply.github.com> Date: Wed, 4 Jun 2025 17:34:54 +0530 Subject: [PATCH 2/7] fix: saperated validations for each purpose of validation (cherry picked from commit 07d1a0ed9c251054d7c129bb082071656ed9a095) # Conflicts: # erpnext/assets/doctype/asset_movement/asset_movement.py --- .../doctype/asset_movement/asset_movement.js | 11 ++- .../doctype/asset_movement/asset_movement.py | 91 ++++++++++++------- 2 files changed, 67 insertions(+), 35 deletions(-) diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.js b/erpnext/assets/doctype/asset_movement/asset_movement.js index e445c90f308..f56c1e31f27 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.js +++ b/erpnext/assets/doctype/asset_movement/asset_movement.js @@ -62,8 +62,8 @@ frappe.ui.form.on("Asset Movement", { fieldnames_to_be_altered = { target_location: { read_only: 0, reqd: 1 }, source_location: { read_only: 1, reqd: 0 }, - from_employee: { read_only: 0, reqd: 0 }, - to_employee: { read_only: 1, reqd: 0 }, + from_employee: { read_only: 1, reqd: 0 }, + to_employee: { read_only: 0, reqd: 0 }, }; } else if (frm.doc.purpose === "Issue") { fieldnames_to_be_altered = { @@ -72,6 +72,13 @@ frappe.ui.form.on("Asset Movement", { from_employee: { read_only: 1, reqd: 0 }, to_employee: { read_only: 0, reqd: 1 }, }; + } else if (frm.doc.purpose === "Transfer and Issue") { + fieldnames_to_be_altered = { + target_location: { read_only: 0, reqd: 1 }, + source_location: { read_only: 0, reqd: 1 }, + from_employee: { read_only: 0, reqd: 1 }, + to_employee: { read_only: 0, reqd: 1 }, + }; } if (fieldnames_to_be_altered) { Object.keys(fieldnames_to_be_altered).forEach((fieldname) => { diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.py b/erpnext/assets/doctype/asset_movement/asset_movement.py index 0c986d086ce..9c5320d6c83 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/asset_movement.py @@ -31,47 +31,42 @@ class AssetMovement(Document): # end: auto-generated types def validate(self): - self.validate_asset() - self.validate_location() - self.validate_employee() - - def validate_asset(self): for d in self.assets: - status, company = frappe.db.get_value("Asset", d.asset, ["status", "company"]) - if self.purpose == "Transfer" and status in ("Draft", "Scrapped", "Sold"): - frappe.throw(_("{0} asset cannot be transferred").format(status)) + self.validate_asset(d) + self.validate_movement(d) - if company != self.company: - frappe.throw(_("Asset {0} does not belong to company {1}").format(d.asset, self.company)) + def validate_asset(self, d): + status, company = frappe.db.get_value("Asset", d.asset, ["status", "company"]) + if self.purpose == "Transfer" and status in ("Draft", "Scrapped", "Sold"): + frappe.throw(_("{0} asset cannot be transferred").format(status)) - 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")) + if company != self.company: + frappe.throw(_("Asset {0} does not belong to company {1}").format(d.asset, self.company)) - def validate_location(self): - for d in self.assets: - if self.purpose in ["Transfer", "Issue"]: - current_location = frappe.db.get_value("Asset", d.asset, "location") - if d.source_location: - if current_location != d.source_location: - frappe.throw( - _("Asset {0} does not belongs to the location {1}").format( - d.asset, d.source_location - ) - ) - else: - d.source_location = current_location + def validate_movement(self, d): + if self.purpose == "Transfer and Issue": + self.validate_location_and_employee(d) + elif self.purpose in ["Receipt", "Transfer"]: + self.validate_location(d) + else: + self.validate_employee(d) - if self.purpose == "Issue": - if d.target_location: + def validate_location_and_employee(self, d): + self.validate_location(d) + self.validate_employee(d) + + def validate_location(self, d): + if self.purpose in ["Transfer", "Transfer and Issue"]: + current_location = frappe.db.get_value("Asset", d.asset, "location") + if d.source_location: + if current_location != d.source_location: frappe.throw( - _( - "Issuing cannot be done to a location. Please enter employee to issue the Asset {0} to" - ).format(d.asset), - title=_("Incorrect Movement Purpose"), + _("Asset {0} does not belongs to the location {1}").format(d.asset, d.source_location) ) - if not d.to_employee: - frappe.throw(_("Employee is required while issuing Asset {0}").format(d.asset)) + else: + d.source_location = current_location +<<<<<<< HEAD if self.purpose == "Transfer": if d.to_employee: frappe.throw( @@ -117,12 +112,42 @@ class AssetMovement(Document): frappe.throw( _("Asset {0} does not belongs to the custodian {1}").format(d.asset, d.from_employee) ) +======= + if not d.target_location: + frappe.throw(_("Target Location is required for transferring Asset {0}").format(d.asset)) + if d.source_location == d.target_location: + frappe.throw(_("Source and Target Location cannot be same")) +>>>>>>> 07d1a0ed9c (fix: saperated validations for each purpose of validation) + if self.purpose == "Receipt": + if not d.target_location: + frappe.throw(_("Target Location is required while receiving Asset {0}").format(d.asset)) if d.to_employee and frappe.db.get_value("Employee", d.to_employee, "company") != self.company: frappe.throw( _("Employee {0} does not belongs to the company {1}").format(d.to_employee, self.company) ) + def validate_employee(self, d): + if self.purpose == "Tranfer and Issue": + if not d.from_employee: + frappe.throw(_("From Employee is required while issuing Asset {0}").format(d.asset)) + + if d.from_employee: + current_custodian = frappe.db.get_value("Asset", d.asset, "custodian") + + if current_custodian != d.from_employee: + frappe.throw( + _("Asset {0} does not belongs to the custodian {1}").format(d.asset, d.from_employee) + ) + + if not d.to_employee: + frappe.throw(_("Employee is required while issuing Asset {0}").format(d.asset)) + + if d.to_employee and frappe.db.get_value("Employee", d.to_employee, "company") != self.company: + frappe.throw( + _("Employee {0} does not belongs to the company {1}").format(d.to_employee, self.company) + ) + def on_submit(self): self.set_latest_location_and_custodian_in_asset() From 1c3ac9c1fd03f93b4ac1890ac97ed3194d7e33b5 Mon Sep 17 00:00:00 2001 From: Khushi Rawat <142375893+khushi8112@users.noreply.github.com> Date: Wed, 4 Jun 2025 19:20:43 +0530 Subject: [PATCH 3/7] refactor: split set_latest_location_and_custodian_in_asset into smaller functions (cherry picked from commit 7e52cb2856cb316ed3daaeefa6258ee7f1cd8f4c) --- .../doctype/asset_movement/asset_movement.py | 100 ++++++++++-------- 1 file changed, 55 insertions(+), 45 deletions(-) diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.py b/erpnext/assets/doctype/asset_movement/asset_movement.py index 9c5320d6c83..245ba22ab14 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/asset_movement.py @@ -155,53 +155,63 @@ class AssetMovement(Document): self.set_latest_location_and_custodian_in_asset() def set_latest_location_and_custodian_in_asset(self): + for d in self.assets: + current_location, current_employee = self.get_latest_location_and_custodian(d.asset) + self.update_asset_location_and_custodian(d.asset, current_location, current_employee) + self.log_asset_activity(d.asset, current_location, current_employee) + + def get_latest_location_and_custodian(self, asset): current_location, current_employee = "", "" cond = "1=1" - for d in self.assets: - args = {"asset": d.asset, "company": self.company} + # latest entry corresponds to current document's location, employee when transaction date > previous dates + # In case of cancellation it corresponds to previous latest document's location, employee + args = {"asset": asset, "company": self.company} + latest_movement_entry = frappe.db.sql( + f""" + SELECT asm_item.target_location, asm_item.to_employee + FROM `tabAsset Movement Item` asm_item + JOIN `tabAsset Movement` asm ON asm_item.parent = asm.name + WHERE + asm_item.asset = %(asset)s AND + asm.company = %(company)s AND + asm.docstatus = 1 AND {cond} + ORDER BY asm.transaction_date DESC + LIMIT 1 + """, + args, + ) - # latest entry corresponds to current document's location, employee when transaction date > previous dates - # In case of cancellation it corresponds to previous latest document's location, employee - latest_movement_entry = frappe.db.sql( - f""" - SELECT asm_item.target_location, asm_item.to_employee - FROM `tabAsset Movement Item` asm_item, `tabAsset Movement` asm - WHERE - asm_item.parent=asm.name and - asm_item.asset=%(asset)s and - asm.company=%(company)s and - asm.docstatus=1 and {cond} - ORDER BY - asm.transaction_date desc limit 1 - """, - args, + if latest_movement_entry: + current_location = latest_movement_entry[0][0] + current_employee = latest_movement_entry[0][1] + + return current_location, current_employee + + def update_asset_location_and_custodian(self, asset_id, location, employee): + asset = frappe.get_doc("Asset", asset_id) + + if employee and employee != asset.custodian: + frappe.db.set_value("Asset", asset_id, "custodian", employee) + if location and location != asset.location: + frappe.db.set_value("Asset", asset_id, "location", location) + + def log_asset_activity(self, asset_id, location, employee): + if location and employee: + add_asset_activity( + asset_id, + _("Asset received at Location {0} and issued to Employee {1}").format( + get_link_to_form("Location", location), + get_link_to_form("Employee", employee), + ), + ) + elif location: + add_asset_activity( + asset_id, + _("Asset transferred to Location {0}").format(get_link_to_form("Location", location)), + ) + elif employee: + add_asset_activity( + asset_id, + _("Asset issued to Employee {0}").format(get_link_to_form("Employee", employee)), ) - - if latest_movement_entry: - current_location = latest_movement_entry[0][0] - current_employee = latest_movement_entry[0][1] - - frappe.db.set_value("Asset", d.asset, "location", current_location, update_modified=False) - frappe.db.set_value("Asset", d.asset, "custodian", current_employee, update_modified=False) - - if current_location and current_employee: - add_asset_activity( - d.asset, - _("Asset received at Location {0} and issued to Employee {1}").format( - get_link_to_form("Location", current_location), - get_link_to_form("Employee", current_employee), - ), - ) - elif current_location: - add_asset_activity( - d.asset, - _("Asset transferred to Location {0}").format( - get_link_to_form("Location", current_location) - ), - ) - elif current_employee: - add_asset_activity( - d.asset, - _("Asset issued to Employee {0}").format(get_link_to_form("Employee", current_employee)), - ) From bde63ed0e50786725759502010b39eb706ab3ed5 Mon Sep 17 00:00:00 2001 From: Khushi Rawat <142375893+khushi8112@users.noreply.github.com> Date: Thu, 5 Jun 2025 13:05:14 +0530 Subject: [PATCH 4/7] fix: failing test case (cherry picked from commit 7d3bec8ef8e1225027a7a4c9d850481bc82dae46) --- erpnext/assets/doctype/asset_movement/test_asset_movement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset_movement/test_asset_movement.py b/erpnext/assets/doctype/asset_movement/test_asset_movement.py index 52590d2ba86..07879acd1f0 100644 --- a/erpnext/assets/doctype/asset_movement/test_asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/test_asset_movement.py @@ -88,7 +88,7 @@ class TestAssetMovement(unittest.TestCase): ) # after issuing, asset should belong to an employee not at a location - self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), None) + self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), "Test Location 2") self.assertEqual(frappe.db.get_value("Asset", asset.name, "custodian"), employee) create_asset_movement( From df938f24d4bee1f8e0b51a69d797a34e5bc18e4c Mon Sep 17 00:00:00 2001 From: Khushi Rawat <142375893+khushi8112@users.noreply.github.com> Date: Wed, 25 Jun 2025 14:47:00 +0530 Subject: [PATCH 5/7] chore: resolved conflicts --- .../doctype/asset_movement/asset_movement.py | 49 ------------------- 1 file changed, 49 deletions(-) diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.py b/erpnext/assets/doctype/asset_movement/asset_movement.py index 245ba22ab14..db4e7510670 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/asset_movement.py @@ -65,59 +65,10 @@ class AssetMovement(Document): ) else: d.source_location = current_location - -<<<<<<< HEAD - if self.purpose == "Transfer": - if d.to_employee: - frappe.throw( - _( - "Transferring cannot be done to an Employee. Please enter location where Asset {0} has to be transferred" - ).format(d.asset), - title=_("Incorrect Movement Purpose"), - ) - if not d.target_location: - frappe.throw( - _("Target Location is required while transferring Asset {0}").format(d.asset) - ) - if d.source_location == d.target_location: - frappe.throw(_("Source and Target Location cannot be same")) - - if self.purpose == "Receipt": - if not (d.source_location) and not (d.target_location or d.to_employee): - frappe.throw( - _("Target Location or To Employee is required while receiving Asset {0}").format( - d.asset - ) - ) - elif d.source_location: - if d.from_employee and not d.target_location: - frappe.throw( - _( - "Target Location is required while receiving Asset {0} from an employee" - ).format(d.asset) - ) - elif d.to_employee and d.target_location: - frappe.throw( - _( - "Asset {0} cannot be received at a location and given to an employee in a single movement" - ).format(d.asset) - ) - - def validate_employee(self): - for d in self.assets: - if d.from_employee: - current_custodian = frappe.db.get_value("Asset", d.asset, "custodian") - - if current_custodian != d.from_employee: - frappe.throw( - _("Asset {0} does not belongs to the custodian {1}").format(d.asset, d.from_employee) - ) -======= if not d.target_location: frappe.throw(_("Target Location is required for transferring Asset {0}").format(d.asset)) if d.source_location == d.target_location: frappe.throw(_("Source and Target Location cannot be same")) ->>>>>>> 07d1a0ed9c (fix: saperated validations for each purpose of validation) if self.purpose == "Receipt": if not d.target_location: From ddbdcbb026acbfc6cbbdbd0ce4506a4d809b8155 Mon Sep 17 00:00:00 2001 From: Khushi Rawat <142375893+khushi8112@users.noreply.github.com> Date: Wed, 25 Jun 2025 14:47:47 +0530 Subject: [PATCH 6/7] chore: resolved conflicts --- erpnext/assets/doctype/asset_movement/asset_movement.json | 8 -------- 1 file changed, 8 deletions(-) diff --git a/erpnext/assets/doctype/asset_movement/asset_movement.json b/erpnext/assets/doctype/asset_movement/asset_movement.json index 1bd1ad83ead..a656acf1265 100644 --- a/erpnext/assets/doctype/asset_movement/asset_movement.json +++ b/erpnext/assets/doctype/asset_movement/asset_movement.json @@ -97,11 +97,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], -<<<<<<< HEAD - "modified": "2023-06-28 16:54:26.571083", -======= "modified": "2025-05-30 17:01:55.864353", ->>>>>>> f5e5146021 (feat: added Transfer and Issue option in purpose) "modified_by": "Administrator", "module": "Assets", "name": "Asset Movement", @@ -154,12 +150,8 @@ "write": 1 } ], -<<<<<<< HEAD - "sort_field": "modified", -======= "row_format": "Dynamic", "sort_field": "creation", ->>>>>>> f5e5146021 (feat: added Transfer and Issue option in purpose) "sort_order": "DESC", "states": [] } From dc642fbc410ff3777fab61f2c8000b44aa570ce2 Mon Sep 17 00:00:00 2001 From: khushi8112 Date: Wed, 25 Jun 2025 15:04:17 +0530 Subject: [PATCH 7/7] chore: linters check --- .../doctype/asset_value_adjustment/asset_value_adjustment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/assets/doctype/asset_value_adjustment/asset_value_adjustment.py b/erpnext/assets/doctype/asset_value_adjustment/asset_value_adjustment.py index 1877f173168..2c1d7d9fcab 100644 --- a/erpnext/assets/doctype/asset_value_adjustment/asset_value_adjustment.py +++ b/erpnext/assets/doctype/asset_value_adjustment/asset_value_adjustment.py @@ -217,6 +217,7 @@ class AssetValueAdjustment(Document): salvage_value_adjustment = (difference_amount * row.salvage_value_percentage) / 100 return flt(salvage_value_adjustment if self.docstatus == 1 else -1 * salvage_value_adjustment) + @frappe.whitelist() def get_value_of_accounting_dimensions(asset_name): dimension_fields = [*frappe.get_list("Accounting Dimension", pluck="fieldname"), "cost_center"]