Skip to content

fix(pos): remove redundant opening balance dialog onchange handler#54591

Open
ravibharathi656 wants to merge 1 commit intofrappe:developfrom
aerele:pos-opening-dialog
Open

fix(pos): remove redundant opening balance dialog onchange handler#54591
ravibharathi656 wants to merge 1 commit intofrappe:developfrom
aerele:pos-opening-dialog

Conversation

@ravibharathi656
Copy link
Copy Markdown
Member

@ravibharathi656 ravibharathi656 commented Apr 28, 2026

Issue:

In the POS Opening Entry dialog, when editing balance_details.opening_amount, the value in the currently edited row resets to 0.

Root Cause:

A custom onChange handler was implemented on the dialog’s child table that manually rewrote the opening_amount field and triggered a full grid refresh. However, the dialog table fields already persist edits using the system’s built in grid control, making this handler redundant.

https://github.com/frappe/frappe/blame/9d683f15c7c0ff73f8dcc7464a20c25530cf43c9/frappe/public/js/frappe/form/controls/base_control.js#L270

Additionally, the handler relied on a stale row context (this), which caused the active row to refresh with an incorrect state, resulting in the value being reset.

Backport needed v16, v15

@ravibharathi656 ravibharathi656 marked this pull request as ready for review April 28, 2026 12:53
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: f039b7db-4b25-480a-a9b2-3967d2e7c753

📥 Commits

Reviewing files that changed from the base of the PR and between 084c7f7 and 825741b.

📒 Files selected for processing (1)
  • erpnext/selling/page/point_of_sale/pos_controller.js
💤 Files with no reviewable changes (1)
  • erpnext/selling/page/point_of_sale/pos_controller.js

📝 Walkthrough

Walkthrough

The pull request removes the per-row onchange event handler for the "Opening Amount" field in the POS opening voucher's balance details table within the pos_controller.js file. Previously, when users edited the opening_amount field in the balance_details table, the handler would manually propagate changes to the underlying table data structure and trigger a grid refresh. This event handler logic is now deleted.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: removing a redundant onchange handler from the POS opening balance dialog, which aligns with the changeset that removed 9 lines of onchange logic.
Description check ✅ Passed The description clearly explains the issue (balance reset bug), root cause (redundant handler with stale context), and the solution (removing the handler), all of which are directly related to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant