Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: refactor Asset Repair and Stock Entry linkage to resolve amendme… #41919

Merged

Conversation

khushi8112
Copy link
Member

This pull request addresses an issue with the linkage between Asset Repair and Stock Entry doctypes that caused errors when amending Asset Repair records. The following changes have been made to resolve the issue and improve the overall functionality:

  • Removed Stock Entry Link from Asset Repair
  • Added Asset Repair Link in Stock Entry

Additional Changes:

  • Moved Warehouse Field to Correct Location: The warehouse field, previously located in the Asset Repair doctype, has been moved to the Stock Items child table where it appropriately belongs.

Additional validations:

  • Completion Date: Added validation to ensure that the completion date is appropriately set and follows logical constraints.
  • Purchase Invoice: Added validation to ensure that a valid purchase invoice is associated where required.

no-docs

purchase_invoice: function (frm) {
if (frm.doc.purchase_invoice) {
frappe.call({
method: "frappe.client.get",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't make sense to return the entire doc here, use frappe.client.get_value instead.

Also, use 'base_net_total' to set it as repair_cost.

@@ -20,15 +20,6 @@ frappe.ui.form.on("Asset Repair", {
};
};

frm.fields_dict.warehouse.get_query = function (doc) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same filter should be applied to the warehouse field in the child table.

@@ -248,8 +248,8 @@ def validate_serial_no(self, stock_item):
)

def increase_stock_quantity(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is no longer required, the framework will throw a validation on cancellation of asset repair, asking to cancel the stock entry first. Let the user cancel the stock entry manually, it is not a good idea to ignore the link while cancelling.

frappe.db.set_value(
"Stock Entry", asset_repair.stock_entry, "asset_repair", asset_repair.name
)
frappe.db.commit()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commit

warehouse = frappe.db.get_value("Asset Repair", item.parent, "warehouse")
if warehouse:
frappe.db.set_value("Asset Repair Consumed Item", item.name, "warehouse", warehouse)
frappe.db.commit()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commit

Comment on lines 6 to 11
items = frappe.get_all("Asset Repair Consumed Item", fields=["name", "parent"])

for item in items:
warehouse = frappe.db.get_value("Asset Repair", item.parent, "warehouse")
if warehouse:
frappe.db.set_value("Asset Repair Consumed Item", item.name, "warehouse", warehouse)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid the for loop and multiple update queries, by using a simple join query (between parent and children) to update the value.

frappe.db.sql("""
	update `tabAsset Repair` ar, `Asset Repair Consumed Item` ar_item
	set ar_item.warehouse = ar.warehouse
	where ar.name = ar_item.parent
		and ifnull(ar.warehouse, '') != ''
""")

Use frappe.qb instead of frappe.db.sql

@@ -674,14 +675,21 @@
{
"fieldname": "column_break_eaoa",
"fieldtype": "Column Break"
},
{
"depends_on": "eval:doc.asset_repair",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it read-only

@nabinhait nabinhait merged commit ba79e68 into frappe:develop Jun 27, 2024
18 checks passed
mergify bot pushed a commit that referenced this pull request Jun 27, 2024
#41919)

* fix: refactor Asset Repair and Stock Entry linkage to resolve amendment issues

* chore: added missing patch to patches.txt

* chore: fixing previous changes

* chore: fixing minor issues

* fix: code changes to enhance efficiency

* chore: replaced frappe.qb with db.sql because of conflict

* fix: minor changes

(cherry picked from commit ba79e68)

# Conflicts:
#	erpnext/assets/doctype/asset_repair/asset_repair.json
#	erpnext/assets/doctype/asset_repair_consumed_item/asset_repair_consumed_item.json
#	erpnext/patches.txt
#	erpnext/stock/doctype/stock_entry/stock_entry.json
nabinhait pushed a commit that referenced this pull request Jul 1, 2024
…… (backport #41919) (#42058)

* fix: refactor Asset Repair and Stock Entry linkage to resolve amendme… (#41919)

* fix: refactor Asset Repair and Stock Entry linkage to resolve amendment issues

* chore: added missing patch to patches.txt

* chore: fixing previous changes

* chore: fixing minor issues

* fix: code changes to enhance efficiency

* chore: replaced frappe.qb with db.sql because of conflict

* fix: minor changes

(cherry picked from commit ba79e68)

# Conflicts:
#	erpnext/assets/doctype/asset_repair/asset_repair.json
#	erpnext/assets/doctype/asset_repair_consumed_item/asset_repair_consumed_item.json
#	erpnext/patches.txt
#	erpnext/stock/doctype/stock_entry/stock_entry.json

* chore: fixed conflicts

* fix: removed unmerged patches

* fix: use f-string instead of format call

---------

Co-authored-by: Khushi Rawat <142375893+khushi8112@users.noreply.github.com>
Poorvi-R-Bhat pushed a commit to Poorvi-R-Bhat/erpnextdev that referenced this pull request Jul 3, 2024
frappe#41919)

* fix: refactor Asset Repair and Stock Entry linkage to resolve amendment issues

* chore: added missing patch to patches.txt

* chore: fixing previous changes

* chore: fixing minor issues

* fix: code changes to enhance efficiency

* chore: replaced frappe.qb with db.sql because of conflict

* fix: minor changes
frappe-pr-bot pushed a commit that referenced this pull request Jul 3, 2024
# [15.29.0](v15.28.2...v15.29.0) (2024-07-03)

### Bug Fixes

* add auto-update for overdue status ([#42105](#42105)) ([317cc03](317cc03))
* add string for translation (backport [#41903](#41903)) ([#41963](#41963)) ([48dc24b](48dc24b))
* always post to tax account heads if LCV is booked ([706a6c1](706a6c1))
* batch picking in pick list based on Stock Settings (backport [#42021](#42021)) ([#42134](#42134)) ([a45f8ca](a45f8ca))
* batch reset while making SABB (backport [#42076](#42076)) ([#42123](#42123)) ([c3f5a49](c3f5a49))
* decimal issue in pick list (backport [#41972](#41972)) ([#41982](#41982)) ([9945a90](9945a90))
* **Delivery Note:** only show permitted actions ([cef6d0d](cef6d0d))
* do not show zero balance stock items in stock balance report (backport [#41958](#41958)) ([#41961](#41961)) ([c10b123](c10b123))
* expense account from item group not fetched (backport [#41957](#41957)) ([#41962](#41962)) ([760b2e2](760b2e2))
* fixed asset value in Fixed Asset Register (backport [#41930](#41930)) ([#42027](#42027)) ([f2feeaf](f2feeaf))
* handle none type object error ([b0aef9e](b0aef9e))
* incorrect against_account upon reposting ([a41577a](a41577a))
* incorrect Difference Amount (backport [#42008](#42008)) ([#42013](#42013)) ([838cc5b](838cc5b))
* incorrect discount on other item ([77f4199](77f4199))
* incorrect dr/cr on Adv Payment against Journals ([4e74257](4e74257))
* incorrect time period in asset depreciation schedule (backport [#41805](#41805)) ([#42043](#42043)) ([cf4d4ba](cf4d4ba))
* lead status filter (backport [#41816](#41816)) ([#42046](#42046)) ([3536a75](3536a75))
* manufacturing date issue in the batch (backport [#42034](#42034)) ([#42037](#42037)) ([a981633](a981633))
* move condition for shipment ([2180239](2180239))
* not able to make purchase return (backport [#42053](#42053)) ([#42055](#42055)) ([8a91bf3](8a91bf3))
* pricing rule with and without 'apply multiple' and priority ([f3aa885](f3aa885))
* provisional entry for non stock items ([d61dab8](d61dab8))
* Re-open allows SO's to be over credit limit ([7fcb0f5](7fcb0f5))
* refactor Asset Repair and Stock Entry linkage to resolve amendme… (backport [#41919](#41919)) ([#42058](#42058)) ([97c49b9](97c49b9))
* reload asset when creating asset depreciation ([7b5d504](7b5d504))
* reposting file attachment permission issue (backport [#42068](#42068)) ([#42075](#42075)) ([1f3374f](1f3374f))
* resolve gl entries duplication in asset purchase workflow (backport [#41845](#41845)) ([#42120](#42120)) ([58e18e2](58e18e2))
* **Sales Order:** only show permitted actions ([a0011c5](a0011c5))
* show zero stock items filter in the stock balance report (backport [#42147](#42147)) ([#42152](#42152)) ([11ebbf2](11ebbf2))
* stock qty validation in SCR (backport [#42124](#42124)) ([#42133](#42133)) ([d9e62fe](d9e62fe))
* Stock Reservation Entry was not getting created (backport [#42033](#42033)) ([#42035](#42035)) ([e278fc6](e278fc6))
* **test:** incorrect field for customer default billing currency ([3b15708](3b15708))
* this.frm.events.update_cost is not a function (backport [#41960](#41960)) ([#41965](#41965)) ([3b4d397](3b4d397))
* timeout error while submitting JV (backport [#42040](#42040)) ([#42099](#42099)) ([a0e06a4](a0e06a4))
* timeout while cancelling LCV (backport [#42030](#42030)) (backport [#42031](#42031)) ([#42032](#42032)) ([068de08](068de08))
* unhide serial no field (backport [#42045](#42045)) ([#42047](#42047)) ([482832f](482832f))
* valuation rate for the legacy batches (backport [#42011](#42011)) ([#42020](#42020)) ([f6be19c](f6be19c))
* Wrong Delete Batch on Purchase Receipt (backport [#42007](#42007)) ([#42012](#42012)) ([68b318a](68b318a))

### Features

* accounting dimension filters in gp report ([fe9dffb](fe9dffb))
* default account head for operating cost (backport [#41985](#41985)) ([#41987](#41987)) ([44c1671](44c1671))
* **gp:** group by cost center ([068ae87](068ae87))
* Turkish Chart Of Accounts (backport [#41756](#41756)) ([#42028](#42028)) ([63b26e6](63b26e6))

### Performance Improvements

* code optimization to handle large asset creation (backport [#42018](#42018)) ([#42025](#42025)) ([c27f272](c27f272))
* dont run queries unnecessarily, improved filters ([#41993](#41993)) ([b59c91a](b59c91a))
* Performance optmization for Purchase Invoice submission (backport [#40263](#40263)) ([#41946](#41946)) ([d396c18](d396c18))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants