fix: post restructure charges capitalization#1239
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds a read-only checkbox field 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lending/loan_management/doctype/loan_repayment/loan_repayment.py`:
- Around line 127-130: The validation error message for payable charges is
stale: when checking self.repayment_type (allowed values "Charge Payment" or
("Charges Waiver", "Charges Capitalization", "Principal Capitalization") with
self.loan_restructure) update the frappe.throw message so it mentions "Principal
Capitalization" as an allowed case (match the allowed list in the if condition)
— locate the check around self.repayment_type in loan_repayment.py and modify
the thrown string passed to frappe.throw to include Principal Capitalization.
In `@lending/loan_management/doctype/loan_restructure/test_loan_restructure.py`:
- Around line 267-269: The test currently queries Sales Invoice by loan,
docstatus and value_date which can match the wrong invoice; update the
frappe.db.get_value call in test_loan_restructure.py (the sales_invoice lookup)
to include the is_post_restructure_charge filter (e.g., add
"is_post_restructure_charge": 1 to the filters) so the assertion targets the
post-restructure invoice deterministically and then assert flt(...) == 0 as
before.
In `@lending/patches/v16_0/add_is_post_restructure_charge_field.py`:
- Around line 10-15: The patch that adds the field with "fieldname":
"is_post_restructure_charge" creates it without the print_hide property, causing
inconsistency with the install-time definition; update the patch that constructs
this field (the dict containing "fieldname": "is_post_restructure_charge",
"label": "Is Post Restructure Charge", "fieldtype": "Check", "insert_after":
"loan_repayment", "read_only": 1) to include "print_hide": 1 so upgraded sites
match the behavior in lending/install.py where the same field is defined with
print_hide=1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d8b0456-2762-4639-ad7c-2a4aaca87149
📒 Files selected for processing (7)
lending/install.pylending/loan_management/doctype/loan_disbursement/loan_disbursement.pylending/loan_management/doctype/loan_repayment/loan_repayment.pylending/loan_management/doctype/loan_restructure/loan_restructure.pylending/loan_management/doctype/loan_restructure/test_loan_restructure.pylending/patches.txtlending/patches/v16_0/add_is_post_restructure_charge_field.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lending/loan_management/doctype/loan_repayment/loan_repayment.py`:
- Around line 1540-1543: The current conditional routes all restructure-linked
"Principal Capitalization" repayments through allocate_charges, but
make_loan_repayment_for_adjustment() can create "Principal Capitalization"
repayments with no payable_charges which causes allocate_charges to produce no
repayment_details and prevents update_demands() from clearing overdue principal;
change the conditional in loan_repayment.py (the block that checks
self.repayment_type and self.loan_restructure) to only call allocate_charges
when there are payable charges to allocate (e.g., require self.payable_charges
or similar non-empty collection) or explicitly exclude "Principal
Capitalization" cases that have no payable_charges so that repayments created by
make_loan_repayment_for_adjustment() bypass allocate_charges and still let
update_demands() update/clear principal demands.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23fd0af9-3ea9-4e34-b55a-1591ff1a4f17
📒 Files selected for processing (1)
lending/loan_management/doctype/loan_repayment/loan_repayment.py
a5c4732 to
6713d25
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1239 +/- ##
===========================================
- Coverage 76.37% 76.33% -0.04%
===========================================
Files 142 142
Lines 10207 10245 +38
===========================================
+ Hits 7796 7821 +25
- Misses 2411 2424 +13
🚀 New features to boost your workflow:
|
fix: post restructure charges capitalization (backport #1239)
Summary by CodeRabbit
New Features
Improvements
Tests
Chores