fix: include accounting dimensions in stock entries created during asset repair.

[ljain112]

Issue: Accounting dimensions are not copied in the stock entry created from the asset repair.
closes: #48854

[coderabbitai]

Warning

Rate limit exceeded

@ljain112 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 4 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 6e3997a and cdbe8b9.

📒 Files selected for processing (1)

• erpnext/assets/doctype/asset_repair/asset_repair.js (2 hunks)

📝 Walkthrough

Walkthrough

This change updates erpnext/assets/doctype/asset_repair/asset_repair.py to import get_accounting_dimensions and build an accounting_dimensions dictionary that merges cost_center, project, and all dynamic accounting dimensions. Those dimensions are spread into each stock item entry created during repair and are propagated into GL entry generation. on_cancel now sets ignore_linked_doctypes to ("GL Entry", "Stock Ledger Entry"); make_gl_entries no longer mutates ignore_linked_doctypes. Additionally, erpnext/assets/doctype/asset_repair/asset_repair.js adds a show_general_ledger(frm) helper that adds an Accounting Ledger button when the document is submitted.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify correct usage of get_accounting_dimensions and that all dynamic dimension field names match expectations.
• Confirm accounting_dimensions are merged without overwriting required per-item fields and are present on created Stock Entry items.
• Validate GL entries receive the same dimensions and no duplicate/omitted fields occur.
• Review on_cancel change to ensure cancellation flows and linked-doc ignore behavior are safe.
• Inspect the new JS helper for correct filter parameters and routing.

Files/areas to focus:

• erpnext/assets/doctype/asset_repair/asset_repair.py
• erpnext/assets/doctype/asset_repair/asset_repair.js
• Stock Entry and GL entry creation paths invoked by asset_repair (spot-check integration points)

Possibly related PRs

• fix: include cost_center and project upon accounting dimension fetch #50339 — Changes around fetching and propagating accounting dimensions (including cost_center/project) into stock and GL flows; likely overlaps with the get_accounting_dimensions usage here.

Suggested reviewers

• khushi8112
• ruthra-kumar
• diptanilsaha

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Out of Scope Changes check	❓ Inconclusive	While the main objective of including accounting dimensions is addressed, the linked issue also mentions establishing a reference/link between Stock Entry and Asset Repair, which is not evident from the code changes provided.	Verify whether the Stock Entry reference/link requirement from issue #48854 was intentionally deferred or if additional changes are needed to fully establish the Asset Repair-Stock Entry connection.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding accounting dimensions to stock entries created during asset repair.
Description check	✅ Passed	The description is related to the changeset, referencing the specific issue being fixed and explaining the core problem.
Linked Issues check	✅ Passed	The changes address both primary objectives from issue #48854: accounting dimensions are now propagated to Stock Entries via the new accounting_dimensions dictionary, and the code handles proper GL entry generation and cancellation.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

erpnext/assets/doctype/asset_repair/asset_repair.py (1)

279-281: Address linter warning by treating ignore_linked_doctypes as a non‑DB flag

The linter complaint about “modifying but not committing” likely comes from setting self.ignore_linked_doctypes directly in make_gl_entries. Since this is intended as a runtime flag (not a persisted field), it’s better practice to attach it to self.flags, which also avoids the “not committed” noise:

-    if cancel:
-        self.ignore_linked_doctypes = ("GL Entry", "Stock Ledger Entry")
+    if cancel:
+        # mark linked doctypes to ignore during cancellation; not a DB field
+        self.flags.ignore_linked_doctypes = ("GL Entry", "Stock Ledger Entry")

This should satisfy the semgrep rule while preserving existing behavior when cancelling GL/stock entries.

🧹 Nitpick comments (1)

erpnext/assets/doctype/asset_repair/asset_repair.py (1)

237-241: Accounting dimensions propagation logic looks correct; consider small refinements

This achieves the PR goal: all accounting dimensions on Asset Repair (including cost_center, project, and dynamic dimensions from get_accounting_dimensions) are now pushed into each Stock Entry item via **accounting_dimensions. That should let Stock Entry / GL logic dimension these rows correctly.

Two minor points you might consider (not blockers):

1. Duplicate keys for standard dimensions
   If get_accounting_dimensions() already includes cost_center / project, they are specified twice in the dict; the later **{dimension: self.get(dimension) ...} just overwrites the earlier explicit values with the same values. Harmless, but you could simplify to rely on one source of truth if desired.

2. Filtering by document type (optional)
   get_accounting_dimensions() returns all enabled dimensions, regardless of document_type. Some of these may not exist on Asset Repair or Stock Entry items and will just be None. That’s generally safe, but you could tighten it by passing a filter like filters={"disabled": 0, "document_type": self.doctype} (or to the relevant Stock Entry item doctype) if you want to avoid carrying irrelevant keys. Based on accounting_dimension.get_accounting_dimensions snippet, this is supported. Based on learnings, this is an optional clean‑up, not a correctness issue.

Functionally, the current implementation is sound and meets the stated requirement.

Also applies to: 254-255

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0520ab3 and 147a5ee.

📒 Files selected for processing (1)

• erpnext/assets/doctype/asset_repair/asset_repair.py (3 hunks)

🧰 Additional context used

🪛 GitHub Actions: Linters

erpnext/assets/doctype/asset_repair/asset_repair.py

[error] 23-33: Blocking rule fired: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting-other-method. self.make_gl_entries is called from self.on_submit; check if changes to self.ignore_linked_doctypes are committed to the database.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: Patch Test
• GitHub Check: Python Unit Tests (3)
• GitHub Check: Python Unit Tests (4)
• GitHub Check: Python Unit Tests (2)
• GitHub Check: Python Unit Tests (1)
• GitHub Check: Summary

🔇 Additional comments (1)

erpnext/assets/doctype/asset_repair/asset_repair.py (1)

10-12: Import of get_accounting_dimensions is appropriate and scoped correctly

The new import is minimal, used only in decrease_stock_quantity, and does not introduce any obvious circular dependency risk in this file. No changes needed here.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7acb7d7 and 6e3997a.

📒 Files selected for processing (1)

• erpnext/assets/doctype/asset_repair/asset_repair.js (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: Patch Test
• GitHub Check: Python Unit Tests (3)
• GitHub Check: Python Unit Tests (4)
• GitHub Check: Python Unit Tests (2)
• GitHub Check: Python Unit Tests (1)
• GitHub Check: Summary

🔇 Additional comments (1)

erpnext/assets/doctype/asset_repair/asset_repair.js (1)

76-76: LGTM! Good refactoring.

Extracting the general ledger button setup into a separate method improves code organization and maintainability.