fix(expense_claim): handle partial advance settlement in expense claims#4563
fix(expense_claim): handle partial advance settlement in expense claims#4563iamkhanraheel wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds a validation rule in the ExpenseClaim.validate method that resets is_paid to 0 when a claim has a positive grand_total and an associated total_advance_amount. The change runs before set_status() is called, affecting how the claim's status is subsequently calculated. This prevents the status from incorrectly resolving to "Paid" in scenarios where advances exist but the claim settlement is incomplete. 🚥 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hrms/hr/doctype/expense_claim/expense_claim.py (1)
114-120:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove
is_paidreset beforeset_status()to make the fix effective.Right now,
self.set_status()(Line 114) runs beforeself.is_paid = 0(Line 120), so status can still be computed as"Paid"using staleis_paid. This defeats the intended partial-advance behavior.Suggested fix
self.calculate_taxes() - self.set_status() + if flt(self.grand_total) > 0 and self.total_advance_amount: + self.is_paid = 0 + self.set_status() self.validate_company_and_department() if self.task and not self.project: self.project = frappe.db.get_value("Task", self.task, "project") - - if flt(self.grand_total) > 0 and self.total_advance_amount: - self.is_paid = 0🤖 Prompt for 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. In `@hrms/hr/doctype/expense_claim/expense_claim.py` around lines 114 - 120, Move the is_paid reset so it runs before status calculation: ensure that self.is_paid is set to 0 (when flt(self.grand_total) > 0 and self.total_advance_amount) prior to calling self.set_status(); update the order in the method so self.is_paid assignment occurs before the call to set_status(), keeping the existing project lookup (self.project = frappe.db.get_value("Task", self.task, "project")) and validate_company_and_department() in their current logical places.
🤖 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.
Outside diff comments:
In `@hrms/hr/doctype/expense_claim/expense_claim.py`:
- Around line 114-120: Move the is_paid reset so it runs before status
calculation: ensure that self.is_paid is set to 0 (when flt(self.grand_total) >
0 and self.total_advance_amount) prior to calling self.set_status(); update the
order in the method so self.is_paid assignment occurs before the call to
set_status(), keeping the existing project lookup (self.project =
frappe.db.get_value("Task", self.task, "project")) and
validate_company_and_department() in their current logical places.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3bb19a2f-4656-4f60-9278-32f4c6483eb6
📒 Files selected for processing (1)
hrms/hr/doctype/expense_claim/expense_claim.py
Reason
claimReimburseBug.mov
Changes Done
claimReimburseFix.mov
Summary by CodeRabbit
Bug Fixes
Tests