Skip to content

fix: make ledger entries submittable and cleanup invalid test submissions#52921

Merged
sagarvora merged 14 commits intofrappe:developfrom
shubhdoshi21:non-submittable-doctype-fix
Mar 12, 2026
Merged

fix: make ledger entries submittable and cleanup invalid test submissions#52921
sagarvora merged 14 commits intofrappe:developfrom
shubhdoshi21:non-submittable-doctype-fix

Conversation

@shubhdoshi21
Copy link
Copy Markdown
Contributor

@shubhdoshi21 shubhdoshi21 commented Feb 24, 2026

Details

This PR standardizes the behavior of transaction-related records by enabling submittability where required, adjusting core ledger logic to handle link validation, and fixing test cases that were incorrectly attempting to submit non-submittable doctypes.

1. Ledger Submittability (Accounts & Stock)
The following DocTypes have been updated with is_submittable: 1:

  • Accounting: GL Entry, Payment Ledger Entry, Account Closing Balance, Advance Payment Ledger Entry.
  • Stock: Stock Ledger Entry.

2. Core Logic Adjustments
Updated ledger creation logic to handle link validation and submittability:

  • Reverse Ledger Entries: Added ignore_links: True when creating reverse (cancelled) ledger entries in general_ledger.py and stock_ledger.py. This is necessary because during cancellation, the new offsetting entries must link back to the parent document (the Voucher), which is already in a "Cancelled" state. This flag prevents CancelledLinkError.
  • Payment Ledger: Similarly, added ignore_links: True in utils.py for Payment Ledger Entry creation during reversal.

3. Test Case Fixes
Identified and resolved several instances where tests were calling .submit() on DocTypes that do not support the submission workflow:

  • Shipping Rule: Removed invalid .submit() in test_shipping_rule.py.
  • Sales Person & Customer: Cleaned up incorrect .submit() calls in test_accounts_receivable.py.
  • Document Naming Rule: Updated test_accounts_controller.py to use .insert() instead of .submit().
  • Stock Ledger Deletion: Updated test_purchase_receipt.py to use frappe.db.delete for Stock Ledger Entry instead of doc.delete(), as the doc.delete() fails for submittable documents with docstatus = 1.

4. Child Table Update Pattern Fix
Child table doctypes in Frappe are not independently submittable — their docstatus mirrors the parent. Several places were incorrectly calling .insert() / .submit() on child doc objects without handling the docstatus validation. All such patterns have been fixed by setting skip_docstatus_validation = True on the child doc before calling .insert() / .submit(), allowing the records to be correctly persisted with the appropriate docstatus.

  • tax_withholding_entry.py: Set skip_docstatus_validation = True on new entries created via frappe.copy_doc before .insert(). Applied to both the partial adjustment path (_adjust_against_old_entries) and the return invoice cancellation path (_handle_return_invoice_cancellation).
  • subcontracting_inward_controller.py: Set skip_docstatus_validation = True before .insert() / .submit() for new Subcontracting Inward Order Received Item and Subcontracting Inward Order Secondary Item child docs.
  • work_order.py: Set skip_docstatus_validation = True on the appended child row before .insert(). Removed the incorrect "docstatus": 1 from the appended dict.
  • pos_invoice.py: Replaced payment.insert() on a standalone child doc with self.append("payments", payment) and a single self.save() (with flag) after the loop.

Comment thread erpnext/accounts/doctype/sales_invoice_payment/sales_invoice_payment.json Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.44%. Comparing base (9a4c776) to head (5dfecc9).
⚠️ Report is 18 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #52921   +/-   ##
========================================
  Coverage    79.43%   79.44%           
========================================
  Files         1171     1171           
  Lines       124538   124568   +30     
========================================
+ Hits         98932    98958   +26     
- Misses       25606    25610    +4     
Files with missing lines Coverage Δ
...rpnext/accounts/doctype/pos_invoice/pos_invoice.py 74.14% <100.00%> (+0.15%) ⬆️
...counts/doctype/shipping_rule/test_shipping_rule.py 97.43% <ø> (-0.07%) ⬇️
...ype/tax_withholding_entry/tax_withholding_entry.py 95.52% <100.00%> (+0.01%) ⬆️
erpnext/accounts/general_ledger.py 92.76% <100.00%> (+0.03%) ⬆️
...rt/accounts_receivable/test_accounts_receivable.py 100.00% <100.00%> (ø)
erpnext/accounts/utils.py 74.41% <100.00%> (+0.02%) ⬆️
...xt/controllers/subcontracting_inward_controller.py 79.55% <100.00%> (+0.15%) ⬆️
...next/controllers/tests/test_accounts_controller.py 99.85% <ø> (ø)
...ext/manufacturing/doctype/work_order/work_order.py 81.59% <100.00%> (+0.01%) ⬆️
.../doctype/purchase_receipt/test_purchase_receipt.py 98.98% <100.00%> (-0.01%) ⬇️
... and 1 more

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shubhdoshi21 shubhdoshi21 requested a review from sagarvora March 11, 2026 05:31
@shubhdoshi21 shubhdoshi21 force-pushed the non-submittable-doctype-fix branch from 52a9fd3 to 33cfe8d Compare March 11, 2026 07:13
@sagarvora sagarvora marked this pull request as ready for review March 12, 2026 10:57
@sagarvora sagarvora changed the title fix: enable submittability for ledger entries and cleanup invalid test submissions fix: make ledger entries submittable and cleanup invalid test submissions Mar 12, 2026
@sagarvora sagarvora merged commit ad8c054 into frappe:develop Mar 12, 2026
14 of 15 checks passed
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 12, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: a738ef66-d761-4e80-9f31-873a8fa55d39

📥 Commits

Reviewing files that changed from the base of the PR and between 9a4c776 and 5dfecc9.

📒 Files selected for processing (16)
  • erpnext/accounts/doctype/account_closing_balance/account_closing_balance.json
  • erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.json
  • erpnext/accounts/doctype/gl_entry/gl_entry.json
  • erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.json
  • erpnext/accounts/doctype/pos_invoice/pos_invoice.py
  • erpnext/accounts/doctype/shipping_rule/test_shipping_rule.py
  • erpnext/accounts/doctype/tax_withholding_entry/tax_withholding_entry.py
  • erpnext/accounts/general_ledger.py
  • erpnext/accounts/report/accounts_receivable/test_accounts_receivable.py
  • erpnext/accounts/utils.py
  • erpnext/controllers/subcontracting_inward_controller.py
  • erpnext/controllers/tests/test_accounts_controller.py
  • erpnext/manufacturing/doctype/work_order/work_order.py
  • erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py
  • erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.json
  • erpnext/stock/stock_ledger.py

📝 Walkthrough

Walkthrough

This pull request makes five DocTypes submittable by adding the is_submittable: 1 flag to their JSON definitions (Account Closing Balance, Advance Payment Ledger Entry, GL Entry, Payment Ledger Entry, and Stock Ledger Entry). Additionally, the code changes introduce flags to bypass document status and link validations in multiple creation and submission paths across the accounts, stock, and manufacturing modules. Several test fixtures are simplified by removing explicit submit() calls, and the POS Invoice payment handling is refactored to use batch creation with a single parent save instead of individual submissions. One test switches from ORM-based deletion to direct database deletion.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

mihir-kandoi pushed a commit to mihir-kandoi/erpnext that referenced this pull request Mar 17, 2026
…ions (frappe#52921)

* fix: enable submittability for ledger entries and cleanup invalid test submissions

* fix: reverted child table submittability

* fix: added ignore_links flag for back gl entry

* fix: add ignore_links for reconcile,cancelled PLE,cancelled SLE

* fix: update test_recreate_stock_ledgers to use db.delete instead of doc.delete

* chore: temporary test against frappe PR 37009

* fix: make Advance Payment Ledger Entry submittable

* refactor: add extra line for create_shipping_rule

* chore: revert temporary test against frappe PR 37009

* fix: use parent doc save with ignore_validate_update_after_submit for child table updates

* chore: temporary test against frappe PR 37009

* chore: revert temporary test against frappe PR 37009

* fix: use skip_docstatus_validation
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants