Fix advance payment#20
Conversation
WalkthroughThis update overhauls the handling of advance payment ledger entries, reconciliation, and related references in ERPNext's accounts module. It introduces new fields to several doctypes and models, refactors reconciliation and ledger update logic, revises how GL entries track advance payments, and updates tests and patch scripts to align with the new structure. UI and CSS changes improve POS item selector layouts and unreconcile dialogs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PaymentEntry
participant GL_Entry
participant AdvancePaymentLedgerEntry
participant Order (PO/SO)
participant Utils
User->>PaymentEntry: Submit Payment Entry
PaymentEntry->>GL_Entry: Build GL Map (with advance voucher fields)
PaymentEntry->>AdvancePaymentLedgerEntry: (No direct call, handled by utilities)
PaymentEntry->>Utils: Reconcile against document
Utils->>AdvancePaymentLedgerEntry: Create/update advance ledger entries
AdvancePaymentLedgerEntry->>Order: On update, update voucher outstanding
PaymentEntry-->>User: Submission complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
erpnext/accounts/utils.py (1)
1919-1926: Consider adding error handlingThe function should handle potential errors when fetching the document or calling
set_total_advance_paid.def update_advance_voucher_outstanding(voucher_type, voucher_no): if voucher_type not in ["Purchase Order", "Sales Order"]: return - ref_doc = frappe.get_lazy_doc(voucher_type, voucher_no) - ref_doc.set_total_advance_paid() + try: + ref_doc = frappe.get_lazy_doc(voucher_type, voucher_no) + ref_doc.set_total_advance_paid() + except Exception: + frappe.log_error( + f"Failed to update advance outstanding for {voucher_type} {voucher_no}", + "Update Advance Voucher Outstanding" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.js(0 hunks)erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.json(0 hunks)erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.py(0 hunks)erpnext/accounts/doctype/advance_payment_ledger_entry/test_advance_payment_ledger_entry.py(0 hunks)erpnext/accounts/doctype/journal_entry/journal_entry.py(1 hunks)erpnext/accounts/doctype/journal_entry_account/journal_entry_account.json(2 hunks)erpnext/accounts/doctype/journal_entry_account/journal_entry_account.py(1 hunks)erpnext/accounts/doctype/payment_entry/payment_entry.py(5 hunks)erpnext/accounts/doctype/payment_entry/test_payment_entry.py(2 hunks)erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json(2 hunks)erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py(1 hunks)erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.json(3 hunks)erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.py(3 hunks)erpnext/accounts/doctype/payment_request/test_payment_request.py(1 hunks)erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py(2 hunks)erpnext/accounts/doctype/unreconcile_payment/unreconcile_payment.py(5 hunks)erpnext/accounts/general_ledger.py(1 hunks)erpnext/accounts/utils.py(16 hunks)erpnext/controllers/accounts_controller.py(1 hunks)erpnext/patches.txt(1 hunks)erpnext/patches/v15_0/create_advance_payment_ledger_records.py(0 hunks)erpnext/patches/v15_0/update_payment_ledger_entries_against_advance_doctypes.py(1 hunks)erpnext/public/js/utils/unreconcile.js(3 hunks)
💤 Files with no reviewable changes (5)
- erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.js
- erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.py
- erpnext/accounts/doctype/advance_payment_ledger_entry/test_advance_payment_ledger_entry.py
- erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.json
- erpnext/patches/v15_0/create_advance_payment_ledger_records.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
erpnext/accounts/doctype/journal_entry/journal_entry.py (2)
erpnext/accounts/utils.py (1)
get_advance_payment_doctypes(2337-2347)erpnext/controllers/accounts_controller.py (2)
get_advance_payment_doctypes(2901-2902)get_gl_dict(1210-1305)
erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.py (1)
erpnext/accounts/utils.py (2)
update_advance_voucher_outstanding(1919-1924)update_voucher_outstanding(1927-1969)
⏰ 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). (1)
- GitHub Check: Patch Test
🔇 Additional comments (45)
erpnext/patches.txt (1)
427-427: Confirm Patch Exists and Implements Expected LogicThe patch file
erpnext/patches/v15_0/update_payment_ledger_entries_against_advance_doctypes.pyis present and defines anexecute()function that:
- Retrieves advance payment doctypes via
get_advance_payment_doctypes().- Early-returns if none are found.
- Uses the Frappe QB to update all
Payment Ledger Entryrecords whoseagainst_voucher_typematches an advance payment doctype:
- Sets
against_voucher_type←voucher_type- Sets
against_voucher_no←voucher_no- Sets
advance_voucher_type← oldagainst_voucher_type- Sets
advance_voucher_no← oldagainst_voucher_noThis aligns with the intended migration from the separate Advance Payment Ledger Entry DocType to enhanced Payment Ledger Entry fields for advance voucher tracking.
No further code changes are required. Please ensure that:
get_advance_payment_doctypes()returns the correct list of doctypes.DOCTYPEconstant in the patch is set to"Payment Ledger Entry".erpnext/accounts/doctype/journal_entry_account/journal_entry_account.py (1)
20-21: LGTM! Type declarations correctly support advance voucher tracking.The new fields
advance_voucher_noandadvance_voucher_typeare properly typed and align with the architectural shift to track advance payment references directly in journal entry accounts. The DynamicLink and Link types are appropriate for voucher reference handling.erpnext/accounts/doctype/payment_entry/test_payment_entry.py (2)
87-87: Test expectation correctly updated for multi-currency scenario.Similar to the previous test case, the expected GL entry correctly references the payment entry (
pe.name) instead of the sales order for the multi-currency USD to INR payment scenario.
55-55: Test expectations align with updated GL entry logicThe revised tests now reference the payment entry (
pe.name) instead of the sales order foragainst_voucher, which matches the refactor moving original voucher references intoadvance_voucher_type/advance_voucher_no.Please verify in the Payment Entry implementation that:
- In
erpnext/accounts/doctype/payment_entry/payment_entry.py, the GL‐entry builder (e.g.make_gl_entries) sets
against_voucher_type = "Payment Entry"and
against_voucher = self.name- Advance‐voucher fields (
advance_voucher_type/advance_voucher_no) capture the original document reference.You can locate and review this logic with:
rg -n "def make_gl_entries" -t py erpnext/accounts/doctype/payment_entry rg -A 5 "advance_voucher" erpnext/accounts/doctype/payment_entryerpnext/accounts/doctype/payment_request/test_payment_request.py (1)
474-474: LGTM! Standardized outstanding amount behavior for sales orders.This change aligns the outstanding amount behavior for sales orders with invoices, where the outstanding amount becomes zero after full allocation. This is a logical improvement that provides consistency across document types.
erpnext/accounts/general_ledger.py (1)
319-320: LGTM! Proper inclusion of advance voucher fields in merge criteria.Adding
advance_voucher_typeandadvance_voucher_noto the merge properties ensures that GL entries with different advance voucher references are not incorrectly merged, preserving advance payment tracking integrity.erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py (2)
367-367: LGTM! Updated test expectations align with new unreconciliation behavior.The changes correctly expect
advance_paidto be reset to 0 after unreconciliation and payment cancellation, which provides cleaner state management and is consistent with the updated advance payment handling logic.Also applies to: 373-373
420-420: LGTM! Consistent advance_paid reset behavior in partial unreconciliation.This change correctly expects the unreconciled sales order's
advance_paidto be reset to 0, maintaining consistency with the updated unreconciliation logic even in partial scenarios.erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.json (4)
23-24: LGTM! Well-positioned advance voucher fields in field order.The placement of the new advance voucher fields in the field order maintains logical grouping with other voucher reference fields.
160-173: LGTM! Well-defined advance voucher field configurations.The field definitions are properly structured with:
- Correct field types (Link and Dynamic Link)
- Appropriate options and linking
- Search indexing enabled for performance
- Clear, descriptive labels
212-212: LGTM! Appropriate row format for dynamic linking.Setting
row_formatto "Dynamic" is correct for doctypes containing Dynamic Link fields, enabling proper UI handling of the advance voucher dynamic linking.
178-178: LGTM! Standard timestamp update for schema changes.The modified timestamp update is expected and appropriate for doctype schema modifications.
erpnext/patches/v15_0/update_payment_ledger_entries_against_advance_doctypes.py (1)
1-27: LGTM! Well-implemented migration patch for Payment Ledger Entry consolidation.This patch correctly handles the migration from the deprecated Advance Payment Ledger Entry system to the consolidated Payment Ledger Entry approach:
- ✅ Proper error handling with early return
- ✅ Efficient QueryBuilder usage for bulk updates
- ✅ Correct field mapping preserving advance voucher references
- ✅ Data integrity maintained during migration
The logic appropriately moves
against_voucher_type/notoadvance_voucher_type/nowhile settingagainst_voucher_type/noto the current voucher, ensuring seamless transition to the new data model.erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py (1)
19-20: LGTM! Type annotations are correctly defined.The new advance voucher fields follow the standard pattern with
advance_voucher_typeas a Link to DocType andadvance_voucher_noas a DynamicLink dependent on it. Both are appropriately nullable.erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json (3)
25-27: Field order correctly updated.The new advance voucher fields are properly added to the field order.
156-172: Well-structured field definitions.Both advance voucher fields are correctly defined:
- Read-only fields appropriate for system-managed data
- Dynamic Link pattern correctly implemented
- Consistent column layout (2 columns each)
- Proper labeling
184-184: Dynamic row format is appropriate.Setting
row_formatto "Dynamic" is correct for a table containing dynamic link fields.erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.py (3)
19-19: Import correctly updated.Adding
update_advance_voucher_outstandingto the existing import is clean and necessary for the new functionality.
35-36: Type annotations follow consistent pattern.The advance voucher field types match the pattern used in other DocTypes in this refactor.
183-188: Advance voucher outstanding update logic is well-implemented.The new conditional block correctly:
- Mirrors the existing voucher outstanding update pattern
- Targets the appropriate advance payment DocTypes (Purchase Order, Sales Order)
- Uses the same flag checks for consistency
- Calls the appropriate utility function
erpnext/public/js/utils/unreconcile.js (2)
45-46: Field name standardization is consistent.The changes from
voucher_type/voucher_notoreference_doctype/reference_namealign the frontend with the backend data structure changes and maintain the same mapping logic.Also applies to: 57-58
72-82: Dialog field definitions correctly updated.The unreconcile dialog fields are properly updated to use the new field names while maintaining user-friendly labels and correct dynamic link configuration.
erpnext/accounts/doctype/journal_entry_account/journal_entry_account.json (3)
35-36: Field order logically organized.Adding the advance voucher fields after
reference_detail_nomaintains good form organization.
267-283: Advance voucher field definitions are well-structured.Both fields follow the established pattern:
- Appropriate field types (Link and Dynamic Link)
- Read-only and no_copy properties are correct for system-managed fields
- Proper dynamic linking configuration
295-295: Dynamic row format correctly applied.Setting the row format to "Dynamic" is necessary for proper rendering of the dynamic link fields.
erpnext/accounts/doctype/journal_entry/journal_entry.py (1)
1181-1236: Well-structured refactoring of GL entry constructionThe refactoring to use a
rowdictionary improves code readability and maintainability. The logic correctly handles advance payment doctypes by appropriately setting the voucher references when needed.erpnext/accounts/doctype/unreconcile_payment/unreconcile_payment.py (5)
47-52: Good refactoring to reduce code duplicationThe delegation to
get_linked_payments_for_docsimplifies the method and promotes code reuse.
66-75: Correct handling of outstanding amount updatesThe code properly updates both regular voucher and advance voucher outstanding amounts, which aligns with the new advance payment handling approach.
82-103: Comprehensive reference counting logicThe updated logic correctly counts both regular payment references and advance voucher references, ensuring all linked payments are accounted for.
125-169: Well-structured query extension for advance paymentsThe additional fields and integration of the advance query ensure comprehensive retrieval of linked payments, including advance allocations.
173-200: Clean implementation of advance payment query logicThe function correctly identifies and aggregates advance payment allocations with appropriate filtering and grouping.
erpnext/controllers/accounts_controller.py (1)
2219-2228: set_total_advance_paid logic is sound—no changes needed
- The
calculate_total_advance_from_ledgerquery filters byple.advance_voucher_no == self.nameand then groups by the same field, so it will return at most one record. Accessingadvance[0]is safe.- Currency formatting and validation are applied at the UI/rendering layer; no in-method formatting is required here.
erpnext/accounts/doctype/payment_entry/payment_entry.py (4)
199-207: Method reordering looks appropriate.Moving
update_payment_requests()beforemake_gl_entries()ensures payment request statuses are updated prior to GL entry creation, which improves data consistency. The removal of advance payment ledger entry calls aligns with the PR's refactoring objectives.
291-309: Consistent cancellation flow maintained.The reordering in
on_cancelmirrors the changes inon_submit, maintaining consistency in the operation sequence during both submission and cancellation.
1442-1464: GL entry logic properly refactored for advance payment tracking.The new conditional logic correctly handles three scenarios:
- Advance payment doctypes (SO/PO) - tracks them as advance vouchers
- Separate party account bookings - maintains proper voucher references
- Standard invoices - preserves existing advance voucher information
This approach effectively replaces the advance payment ledger entry mechanism with inline tracking.
1586-1620: Advance GL entries correctly updated with voucher tracking fields.Both GL entries in the advance reconciliation pair now include
advance_voucher_typeandadvance_voucher_nofields, maintaining the complete audit trail for advance payments without requiring a separate ledger entry doctype.erpnext/accounts/utils.py (9)
675-686: LGTM! Advance voucher fields properly handledThe implementation correctly sets advance voucher fields when the reference type is an advance payment doctype and returns the new row for further processing.
705-765: LGTM! Payment entry references updated correctlyThe function properly handles advance payment doctypes and correctly sets advance voucher fields when processing payment entry references.
942-961: LGTM! Advance voucher fields cleared properlyThe implementation correctly clears advance voucher fields in Payment Ledger Entries when references are removed, with appropriate filtering conditions.
996-999: LGTM! Smart preservation of advance payment referencesThe implementation cleverly preserves advance payment information by transferring advance voucher fields to main reference fields when unlinking, ensuring continuity of advance payment tracking.
1060-1086: LGTM! Consistent handling of advance payment references in PEThe implementation maintains consistency with the JV handling, properly preserving advance payment references with appropriate NULL value handling.
1860-1896: LGTM! Payment ledger entries include advance voucher fieldsThe function properly handles the new advance voucher fields and correctly sets the against voucher values based on the GL entry data.
1928-1970: LGTM! Improved efficiency with early returnsThe function has been optimized with early return patterns and properly handles outstanding amount updates with appropriate precision handling.
1973-2001: LGTM! Robust handling of advance voucher fields in delink operationThe function properly handles NULL values using IfNull for advance voucher fields and includes appropriate early return for safety.
517-519: Potential variable reference issue in advance GL entries processingThe code uses
entryin the method call, but this references the last value from the previous loop (line 489). Should this berowinstead?if voucher_type == "Payment Entry" and doc.book_advance_payments_in_separate_party_account: for row in reposting_rows: - doc.make_advance_gl_entries(entry=row) + doc.make_advance_gl_entries(entry=row)Consider verifying if the parameter name should match the loop variable.
Likely an incorrect or invalid review comment.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
erpnext/controllers/accounts_controller.py (1)
2203-2217: Remove redundant groupby clauseThe
groupby(ple.advance_voucher_no)clause on line 2214 is redundant since you're already filtering byadvance_voucher_no == self.name. This will only return entries for one specific voucher.
🧹 Nitpick comments (5)
erpnext/accounts/doctype/journal_entry/journal_entry.py (1)
1189-1226: Consider adding validation for advance voucher fields.The consolidated
rowdictionary structure improves code maintainability. However, consider adding validation to ensureadvance_voucher_typeandadvance_voucher_noare only set together (both present or both empty) to maintain data integrity.Add validation after the row dictionary creation:
row = { # ... existing fields ... "advance_voucher_type": d.advance_voucher_type, "advance_voucher_no": d.advance_voucher_no, } + +# Validate advance voucher fields are set together +if bool(row.get("advance_voucher_type")) != bool(row.get("advance_voucher_no")): + frappe.throw(_("Row {0}: Both advance voucher type and number must be specified together").format(d.idx))erpnext/accounts/doctype/unreconcile_payment/unreconcile_payment.py (2)
60-75: Enhanced outstanding update logic handles both regular and advance payments.The explicit calls to both
update_voucher_outstandingandupdate_advance_voucher_outstandingensure proper handling of the new advance payment tracking system.Consider adding error handling for the outstanding update operations:
# update outstanding amounts -update_voucher_outstanding( - alloc.reference_doctype, - alloc.reference_name, - alloc.account, - alloc.party_type, - alloc.party, -) -update_advance_voucher_outstanding(alloc.reference_doctype, alloc.reference_name) +try: + update_voucher_outstanding( + alloc.reference_doctype, + alloc.reference_name, + alloc.account, + alloc.party_type, + alloc.party, + ) + update_advance_voucher_outstanding(alloc.reference_doctype, alloc.reference_name) +except Exception as e: + frappe.log_error(f"Failed to update outstanding for {alloc.reference_name}: {str(e)}") + raise
173-200: Well-structured query for advance payment allocations.The
get_advance_queryfunction correctly constructs a query to fetch advance voucher allocations with proper grouping and filtering.Consider using a more explicit condition for the advance voucher type check:
-(IfNull(ple.advance_voucher_type, "") != ""), +(ple.advance_voucher_type.isnotnull() & (ple.advance_voucher_type != "")),This makes the intent clearer and avoids the
IfNullconversion.erpnext/accounts/doctype/payment_entry/payment_entry.py (2)
1437-1438: Add advance voucher fields conditionallyConsider adding the
advance_voucher_typeandadvance_voucher_nofields to the GL dict only when they have values to avoid storing empty fields in GL entries.- "advance_voucher_type": d.advance_voucher_type, - "advance_voucher_no": d.advance_voucher_no, + **({"advance_voucher_type": d.advance_voucher_type} if d.advance_voucher_type else {}), + **({"advance_voucher_no": d.advance_voucher_no} if d.advance_voucher_no else {}),
1586-1587: Apply consistent conditional field additionSimilar to the previous comment, consider adding these fields conditionally to avoid storing empty values.
# For lines 1586-1587: - "advance_voucher_type": invoice.advance_voucher_type, - "advance_voucher_no": invoice.advance_voucher_no, + **({"advance_voucher_type": invoice.advance_voucher_type} if invoice.advance_voucher_type else {}), + **({"advance_voucher_no": invoice.advance_voucher_no} if invoice.advance_voucher_no else {}), # For lines 1612-1613: - "advance_voucher_type": invoice.advance_voucher_type, - "advance_voucher_no": invoice.advance_voucher_no, + **({"advance_voucher_type": invoice.advance_voucher_type} if invoice.advance_voucher_type else {}), + **({"advance_voucher_no": invoice.advance_voucher_no} if invoice.advance_voucher_no else {}),Also applies to: 1612-1613
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.js(0 hunks)erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.json(0 hunks)erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.py(0 hunks)erpnext/accounts/doctype/advance_payment_ledger_entry/test_advance_payment_ledger_entry.py(0 hunks)erpnext/accounts/doctype/journal_entry/journal_entry.py(1 hunks)erpnext/accounts/doctype/journal_entry_account/journal_entry_account.json(2 hunks)erpnext/accounts/doctype/journal_entry_account/journal_entry_account.py(1 hunks)erpnext/accounts/doctype/payment_entry/payment_entry.py(5 hunks)erpnext/accounts/doctype/payment_entry/test_payment_entry.py(2 hunks)erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json(2 hunks)erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py(1 hunks)erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.json(3 hunks)erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.py(3 hunks)erpnext/accounts/doctype/payment_request/test_payment_request.py(1 hunks)erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py(2 hunks)erpnext/accounts/doctype/unreconcile_payment/unreconcile_payment.py(5 hunks)erpnext/accounts/general_ledger.py(1 hunks)erpnext/accounts/utils.py(16 hunks)erpnext/controllers/accounts_controller.py(1 hunks)erpnext/patches.txt(1 hunks)erpnext/patches/v15_0/create_advance_payment_ledger_records.py(0 hunks)erpnext/patches/v15_0/update_payment_ledger_entries_against_advance_doctypes.py(1 hunks)erpnext/public/js/utils/unreconcile.js(3 hunks)
💤 Files with no reviewable changes (5)
- erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.js
- erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.py
- erpnext/patches/v15_0/create_advance_payment_ledger_records.py
- erpnext/accounts/doctype/advance_payment_ledger_entry/advance_payment_ledger_entry.json
- erpnext/accounts/doctype/advance_payment_ledger_entry/test_advance_payment_ledger_entry.py
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: ljain112
PR: ljain112/erpnext#20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/payment_entry/test_payment_entry.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/patches.txt (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.json (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/patches/v15_0/update_payment_ledger_entries_against_advance_doctypes.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/unreconcile_payment/unreconcile_payment.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/payment_entry/payment_entry.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/doctype/journal_entry/journal_entry.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/controllers/accounts_controller.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
erpnext/accounts/utils.py (1)
Learnt from: ljain112
PR: #20
File: erpnext/controllers/accounts_controller.py:2203-2217
Timestamp: 2025-07-26T07:46:45.922Z
Learning: In ERPNext Payment Ledger Entry, advance payment amounts are stored as negative values, but when calculating total advance paid for display purposes, they should be converted to positive values using Abs(Sum(amount)) to show meaningful totals to users.
🧬 Code Graph Analysis (1)
erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.py (1)
erpnext/accounts/utils.py (2)
update_advance_voucher_outstanding(1919-1924)update_voucher_outstanding(1927-1969)
🔇 Additional comments (40)
erpnext/patches.txt (1)
427-427: LGTM! Patch addition aligns with the refactoring objectives.The new patch
update_payment_ledger_entries_against_advance_doctypesis appropriate for migrating existing Payment Ledger Entry records to use the new advance voucher reference fields, consistent with removing the Advance Payment Ledger Entry DocType.erpnext/accounts/doctype/journal_entry_account/journal_entry_account.py (1)
20-21: LGTM! Type declarations are correctly implemented.The new
advance_voucher_typeandadvance_voucher_nofields are properly typed as optional Link and DynamicLink respectively, which is appropriate for tracking advance payment references in journal entry accounts.erpnext/accounts/doctype/payment_entry/test_payment_entry.py (2)
55-55: LGTM! Test expectation correctly updated for new GL entry referencing.The expected GL entry now correctly references the payment entry itself (
pe.name) as theagainst_voucher, which aligns with the refactoring where advance voucher details are tracked separately in dedicated advance voucher fields.
87-87: LGTM! Consistent test update for multi-currency payment scenario.The GL entry expectation is correctly updated to reference the payment entry (
pe.name) instead of the sales order, maintaining consistency with the new advance payment tracking approach.erpnext/accounts/doctype/payment_request/test_payment_request.py (1)
474-474: LGTM! Test reflects updated advance payment logic.This change correctly updates the test assertion to expect zero outstanding amount after full payment allocation to a sales order, which aligns with the new advance payment handling that treats orders consistently with invoices after allocation.
erpnext/accounts/general_ledger.py (1)
319-320: LGTM! Properly extends GL entry merging logic for advance voucher tracking.Adding
advance_voucher_typeandadvance_voucher_noto the merge properties ensures that GL entries with different advance voucher references remain distinct during the merging process, which is essential for the new advance payment tracking system.erpnext/accounts/doctype/unreconcile_payment/test_unreconcile_payment.py (3)
367-367: LGTM! Correctly clears advance payment on unreconciliation.This change properly reflects that when a payment is unreconciled from a sales order, the
advance_paidamount should be reset to zero rather than retaining the previously allocated amount.
373-373: LGTM! Maintains cleared advance payment after cancellation.The assertion correctly verifies that after cancelling the payment entry, the advance payment amount remains cleared (zero) on the sales order.
420-420: LGTM! Correctly clears advance payment for unreconciled order.This change properly verifies that when one of multiple sales orders is unreconciled, only the unreconciled order's
advance_paidis cleared to zero while the other order retains its advance payment amount.erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json (1)
26-27: LGTM! Properly adds advance voucher tracking fields to DocType.The addition of
advance_voucher_typeandadvance_voucher_nofields follows ERPNext conventions with appropriate field types (Link and Dynamic Link), read-only properties, and column widths. Therow_format: "Dynamic"property correctly supports the Dynamic Link functionality. These changes enable direct tracking of advance voucher references within Payment Entry Reference records.Also applies to: 157-172, 184-184
erpnext/patches/v15_0/update_payment_ledger_entries_against_advance_doctypes.py (1)
8-27: LGTM! Well-implemented data migration patch.This patch correctly migrates existing Payment Ledger Entry records to the new advance voucher tracking system. The logic properly:
- Preserves original advance document references in the new
advance_voucher_typeandadvance_voucher_nofields- Updates
against_voucherfields to point to the payment document itself- Uses efficient bulk update with proper error handling
- Follows ERPNext patch conventions
The field mapping transformation is logically sound and maintains data integrity during the migration from the separate Advance Payment Ledger Entry system to the consolidated approach.
erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py (1)
19-20: LGTM! Type annotations correctly updated for advance voucher tracking.The addition of
advance_voucher_noandadvance_voucher_typefields aligns with the PR's objective to consolidate advance payment tracking into existing DocTypes.erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.json (1)
23-24: LGTM! JSON field definitions properly configured for advance voucher tracking.The new fields are correctly defined:
advance_voucher_typeas Link to DocType with search indexingadvance_voucher_noas Dynamic Link with proper options reference- Both fields have appropriate search indexing enabled
- Row format set to "Dynamic" is consistent with dynamic link usage
Also applies to: 160-173, 212-212
erpnext/accounts/doctype/payment_ledger_entry/payment_ledger_entry.py (2)
19-19: LGTM! Import and type annotations correctly added.The import of
update_advance_voucher_outstandingand the new type annotations for advance voucher fields are properly implemented.Also applies to: 35-36
183-188: LGTM! Outstanding update logic correctly implemented.The new conditional block properly mirrors the existing pattern for updating voucher outstanding amounts. The conditions are appropriate:
- Only applies to Purchase Order and Sales Order advance vouchers
- Respects the
update_outstandingflag- Excludes reverse depreciation entries
erpnext/public/js/utils/unreconcile.js (2)
45-46: LGTM! Selection map correctly updated to use standardized reference fields.The changes from
voucher_type/voucher_notoreference_doctype/reference_namemaintain functionality while standardizing field naming across the system.Also applies to: 57-58
72-72: LGTM! Dialog field definitions properly updated.The field definitions correctly use the new standardized reference field names, and the Dynamic Link options reference is properly updated to match.
Also applies to: 80-80, 82-82
erpnext/accounts/doctype/journal_entry_account/journal_entry_account.json (1)
35-36: LGTM! JSON field definitions properly configured for journal entry advance voucher tracking.The new fields are correctly defined:
- Both fields are appropriately marked as
read_onlyandno_copyadvance_voucher_typeas Link to DocType is correctadvance_voucher_noas Dynamic Link with proper options reference- Row format set to "Dynamic" is consistent with dynamic link usage
- Field positioning is logical within the reference section
Also applies to: 268-283, 295-295
erpnext/accounts/doctype/journal_entry/journal_entry.py (2)
194-201: LGTM! Removal of advance payment ledger methods aligns with the PR objectives.The removal of
make_advance_payment_ledger_entries()andupdate_advance_paid()calls is consistent with the PR's goal to eliminate the Advance Payment Ledger Entry DocType and manage advances within the Payment Ledger Entry instead.Also applies to: 283-303
1181-1236: Well-implemented advance payment reference handling.The refactoring correctly transitions from the old advance payment ledger system to the new approach by:
- Using
get_advance_payment_doctypes()to identify advance payment documents- Setting
against_voucher_type/against_voucherto the current journal entry for proper GL tracking- Preserving the original advance reference in
advance_voucher_type/advance_voucher_nofieldsThis maintains the necessary linkages for reconciliation while eliminating the need for a separate advance payment ledger.
erpnext/accounts/doctype/unreconcile_payment/unreconcile_payment.py (3)
10-10: LGTM! Appropriate imports for the new functionality.The addition of
IfNullfor SQL query construction andupdate_advance_voucher_outstandingfor handling advance payment updates aligns with the PR's objectives.Also applies to: 16-16
47-52: Good refactoring to centralize allocation retrieval logic.The delegation to
get_linked_payments_for_docimproves code reusability and maintainability while preserving the original functionality.
82-103: Comprehensive reference counting for advance payments.The updated logic correctly handles both regular allocations and advance payment references by:
- Counting entries that reference different vouchers
- Counting self-referencing entries with advance voucher information
This ensures accurate reference detection for the unreconciliation process.
erpnext/controllers/accounts_controller.py (1)
2219-2228: LGTM!The simplified implementation correctly calculates and sets the total advance paid amount.
erpnext/accounts/doctype/payment_entry/payment_entry.py (5)
199-207: Good: Improved error handling by moving payment request updates earlierMoving
update_payment_requests()before GL entries ensures payment requests are updated even if GL creation fails, providing better error recovery. The removal of advance payment ledger entry calls aligns with the PR's objective of removing that DocType.
291-309: LGTM: Consistent with advance payment ledger removalThe removal of "Advance Payment Ledger Entry" from
ignore_linked_doctypesand movingupdate_payment_requestsbefore GL cancellation maintains consistency with the overall refactoring approach.
1444-1458: Well-structured advance payment handlingThe new logic properly tracks advance payments by using the payment entry as the against voucher while preserving the original advance reference in dedicated fields. This maintains accounting integrity while simplifying the ledger structure.
1-3636: Well-executed refactoring of advance payment handlingThe removal of the Advance Payment Ledger Entry DocType and its replacement with enhanced Payment Ledger Entry tracking is well-implemented. The new approach using
advance_voucher_typeandadvance_voucher_nofields maintains referential integrity while simplifying the system architecture.Key improvements:
- Cleaner GL entry structure
- Better error handling with relocated
update_payment_requests()calls- Consistent tracking of advance references
Consider the suggested optimizations for conditional field addition to keep GL entries clean.
1772-1772: Verify advance-paid calculation implementationI couldn’t find any replacement for the removed
update_advance_paidmethod or any new logic that aggregates advance payments. Please manually confirm that:
- Advance payments are correctly recorded (as negative amounts) in Payment Ledger Entry records.
- A corresponding routine (e.g., in
erpnext/controllers/accounts_controller.py) appliesAbs(Sum(amount))when displaying total advance paid.- All affected doctypes (Payment Entry, Sales Order, etc.) reflect the updated advance-paid values.
erpnext/accounts/utils.py (11)
14-14: LGTM!The
IfNullimport is appropriately added and used for null-safe filtering in database queries throughout the file.
483-540: Well-structured refactoring of reconciliation logic.The changes properly handle the transition from Advance Payment Ledger Entry to enhanced Payment Ledger Entry by:
- Collecting referenced rows during updates for batch processing
- Calling
make_advance_gl_entriesfor Payment Entries with separate advance accounts- Properly rebuilding GL/PL entries for other voucher types
The restructured flow is cleaner and aligns with the PR objectives.
675-687: Proper handling of advance voucher references.The addition correctly preserves advance payment information by setting
advance_voucher_typeandadvance_voucher_nofields when the reference is an advance payment doctype. Returning the new row enables proper downstream processing.
705-765: Consistent advance voucher handling in payment entries.The changes correctly preserve advance payment references by transferring
advance_voucher_typeandadvance_voucher_nofrom existing references to new rows, maintaining referential integrity during reconciliation.
942-961: Proper cleanup of advance voucher references.The addition correctly clears
advance_voucher_typeandadvance_voucher_nofields when removing references, with appropriate filtering to only affect self-referencing entries (wherevoucher_no == against_voucher_no).
996-999: Smart preservation of advance references during unlinking.The change from nullifying references to transferring advance voucher fields ensures that advance payment information is preserved when unlinking journal entries. This maintains the audit trail and reference integrity.
1060-1087: Consistent advance reference preservation in payment entries.The implementation correctly mirrors the journal entry logic, transferring advance voucher fields to main reference fields before clearing them. The use of
IfNullfor null-safe comparison is appropriate.
1860-1896: Comprehensive payment ledger entry creation with advance tracking.The changes properly handle:
- Setting against voucher fields with appropriate fallbacks
- Including advance voucher fields for tracking advance payments
- Correctly setting the delinked status based on the cancel parameter
This ensures complete tracking of advance payment references in the payment ledger.
1919-1926: Well-designed function for advance voucher outstanding updates.The new function appropriately handles outstanding updates for Purchase Orders and Sales Orders by calling
set_total_advance_paid. The early return pattern for unsupported voucher types is clean and efficient.
1928-1970: Improved function structure with better validation.The refactoring adds proper parameter validation and early returns, making the function more robust and efficient. The reorganized logic is cleaner and easier to follow.
1972-2001: Robust delink logic with proper null handling.The improvements include:
- Defensive early return for None input
- Null-safe comparisons using
IfNullfor advance voucher fields- Better query structure and readability
- Proper handling of partial cancellations
These changes make the function more reliable and handle edge cases appropriately.
ljain112
force-pushed
the
fix-advance-payment
branch
from
6ba3e9d to
b5a87ed
Compare
ljain112
force-pushed
the
fix-advance-payment
branch
from
d6af245 to
2a95f3a
Compare
revert: do not save when set email campaign status
fix: attribute error in payment entry
which overrides with 'Serial and Batch Bundle'
…o_cancellation fix: unable to cancel PO if unreconciliation is done
fix: serial no warehouse for backdated stock reco
fix(pick list): make warehouse editable
fix: include empty values in user permission
…list_view feat: list view on item selector in pos
fix: set mandatory field for pos search fields
fix: change modified timestamp so migrations work
Update CODEOWNERS
ljain112
force-pushed
the
fix-advance-payment
branch
from
d204339 to
6da0e1e
Compare
9 participants
Issue
Steps to Replicate the issue:
You will Get error “you cannot create or cancel any accounting entries with in the closed Accounting Period of March”.System dosen't allow to reconcile because for reconcillation of advances whole voucher needs to reposted.
Fix
GL entries for Advance payment
BEFORE
Payment Ledger Entry:
AFTER
Payment Ledger Entry:
Todo
Frappe Support Issue: https://support.frappe.io/app/hd-ticket/38289
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores