-
Notifications
You must be signed in to change notification settings - Fork 10.1k
feat: add default-age-range in accounts settings #51458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #51458 +/- ##
===========================================
+ Coverage 79.09% 79.11% +0.01%
===========================================
Files 1179 1179
Lines 121321 121356 +35
===========================================
+ Hits 95964 96007 +43
+ Misses 25357 25349 -8
🚀 New features to boost your workflow:
|
a1bae62 to
4afe75a
Compare
📝 WalkthroughWalkthroughAdded a new DocType field Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-12-16T05:33:58.723ZApplied to files:
⏰ 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). (6)
🔇 Additional comments (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
erpnext/accounts/report/accounts_receivable/accounts_receivable.js (1)
196-200: Consider checking if the user has already modified the filter.The async fetch could complete after the user has manually set a value, potentially overwriting their input. Consider checking if the filter value differs from the default before setting it.
💡 Suggested improvement
frappe.db.get_single_value("Accounts Settings", "default_ageing_range").then((value) => { if (value) { - report.set_filter_value("range", value); + const current_value = report.get_filter_value("range"); + // Only set if user hasn't changed from the hardcoded default + if (current_value === "30, 60, 90, 120") { + report.set_filter_value("range", value); + } } });erpnext/accounts/report/accounts_payable/accounts_payable.js (1)
169-173: Consider checking if the user has already modified the filter.Similar to the Accounts Receivable report, this async fetch could overwrite user input. Consider checking if the filter value differs from the default before setting it.
💡 Suggested improvement
frappe.db.get_single_value("Accounts Settings", "default_ageing_range").then((value) => { if (value) { - report.set_filter_value("range", value); + const current_value = report.get_filter_value("range"); + if (current_value === "30, 60, 90, 120") { + report.set_filter_value("range", value); + } } });erpnext/accounts/report/accounts_receivable_summary/accounts_receivable_summary.js (1)
141-145: Consider checking if the user has already modified the filter.This async fetch could overwrite user input if they've already set a value. Consider checking if the filter differs from the default before applying the fetched value.
💡 Suggested improvement
frappe.db.get_single_value("Accounts Settings", "default_ageing_range").then((value) => { if (value) { - report.set_filter_value("range", value); + const current_value = report.get_filter_value("range"); + if (current_value === "30, 60, 90, 120") { + report.set_filter_value("range", value); + } } });erpnext/accounts/doctype/accounts_settings/accounts_settings.json (1)
654-659: Add description for the ageing range field and consider validation for user guidance.While consuming reports handle invalid input gracefully via silent filtering, the field lacks documentation and validation that would help users provide correct input:
Missing description: Users need guidance on expected format (comma-separated integers), unit (days), and whether spaces are allowed (currently the code handles both "30, 60, 90" and "30,60,90").
No validation: Without server-side validation, invalid input is silently ignored rather than flagged to the user. This can lead to confusion when the ageing range differs from what users intended to set.
Add a description field to guide users on the expected format:
{ "default": "30, 60, 90, 120", + "description": "Comma-separated aging buckets in days (e.g., 30, 60, 90, 120). Used as default ranges for Accounts Receivable and Accounts Payable reports.", "fieldname": "default_ageing_range", "fieldtype": "Data", "label": "Default Ageing Range" }Consider adding optional validation in
accounts_settings.pyto alert users when invalid values are provided.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
erpnext/accounts/doctype/accounts_settings/accounts_settings.jsonerpnext/accounts/doctype/accounts_settings/accounts_settings.pyerpnext/accounts/report/accounts_payable/accounts_payable.jserpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.jserpnext/accounts/report/accounts_receivable/accounts_receivable.jserpnext/accounts/report/accounts_receivable_summary/accounts_receivable_summary.js
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: diptanilsaha
Repo: frappe/erpnext PR: 49373
File: erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts_accounts_receivable.html:86-96
Timestamp: 2025-08-29T12:08:57.357Z
Learning: In the Process Statement of Accounts accounts receivable template, the summary table that appears when show_future_payments is enabled correctly includes an "Age" column header that displays aggregate age metrics alongside the aging bucket totals. This alignment is intentional and should not be removed.
📚 Learning: 2025-08-29T12:08:57.357Z
Learnt from: diptanilsaha
Repo: frappe/erpnext PR: 49373
File: erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts_accounts_receivable.html:86-96
Timestamp: 2025-08-29T12:08:57.357Z
Learning: In the Process Statement of Accounts accounts receivable template, the summary table that appears when show_future_payments is enabled correctly includes an "Age" column header that displays aggregate age metrics alongside the aging bucket totals. This alignment is intentional and should not be removed.
Applied to files:
erpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.jserpnext/accounts/doctype/accounts_settings/accounts_settings.jsonerpnext/accounts/report/accounts_receivable_summary/accounts_receivable_summary.jserpnext/accounts/report/accounts_payable/accounts_payable.jserpnext/accounts/report/accounts_receivable/accounts_receivable.js
📚 Learning: 2025-11-14T04:52:35.361Z
Learnt from: vorasmit
Repo: frappe/erpnext PR: 49098
File: erpnext/accounts/doctype/financial_report_row/financial_report_row.json:118-124
Timestamp: 2025-11-14T04:52:35.361Z
Learning: In ERPNext Financial Report Row (erpnext/accounts/doctype/financial_report_row), when data_source is "Account Data" and advanced_filtering is false, the filters_editor field provides a UI that automatically populates the calculation_formula field behind the scenes. The calculation_formula field can be hidden but mandatory in this case because it gets set automatically before validation occurs.
Applied to files:
erpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.js
📚 Learning: 2025-09-18T06:20:52.069Z
Learnt from: diptanilsaha
Repo: frappe/erpnext PR: 49608
File: erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts_accounts_receivable.html:331-344
Timestamp: 2025-09-18T06:20:52.069Z
Learning: The Process Statement of Accounts does not pass the `show_sales_person` filter to the Accounts Receivable report, so `filters.show_sales_person` will always be false/undefined in the Process Statement of Accounts template context.
Applied to files:
erpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.jserpnext/accounts/report/accounts_receivable_summary/accounts_receivable_summary.jserpnext/accounts/report/accounts_payable/accounts_payable.jserpnext/accounts/report/accounts_receivable/accounts_receivable.js
📚 Learning: 2025-12-16T05:33:58.723Z
Learnt from: Abdeali099
Repo: frappe/erpnext PR: 51078
File: erpnext/accounts/doctype/financial_report_template/financial_report_engine.py:486-491
Timestamp: 2025-12-16T05:33:58.723Z
Learning: In ERPNext/Frappe codebase, query.run(as_dict=True) returns frappe._dict objects that support both dict-style access (obj["key"]) and attribute-style access (obj.key). Therefore, attribute access on query results is valid and will not raise AttributeError. When reviewing Python code, prefer attribute access (obj.key) for readability where the key is known to exist, but ensure existence checks or fallback handling if there is any doubt about missing keys.
Applied to files:
erpnext/accounts/doctype/accounts_settings/accounts_settings.py
⏰ 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: Summary
🔇 Additional comments (2)
erpnext/accounts/doctype/accounts_settings/accounts_settings.py (1)
43-43: LGTM!The type hint addition is correctly placed and follows the existing pattern for other fields in the AccountsSettings DocType.
erpnext/accounts/doctype/accounts_settings/accounts_settings.json (1)
94-94: LGTM: Logical field placement.The field is appropriately positioned within the Accounts Receivable/Payable Tuning section, which aligns with its purpose of providing default aging ranges for AR/AP reports.
erpnext/accounts/report/accounts_payable_summary/accounts_payable_summary.js
Outdated
Show resolved
Hide resolved
feat: add default-age-range in accounts settings (cherry picked from commit f8f82cc) # Conflicts: # erpnext/accounts/doctype/accounts_settings/accounts_settings.json
feat: add default-age-range in accounts settings (cherry picked from commit f8f82cc) # Conflicts: # erpnext/accounts/doctype/accounts_settings/accounts_settings.json
…51531) Merge pull request #51458 from aerele/default-age-range feat: add default-age-range in accounts settings (cherry picked from commit f8f82cc) # Conflicts: # erpnext/accounts/doctype/accounts_settings/accounts_settings.json Co-authored-by: Sowmya <[email protected]>
# [15.94.0](v15.93.2...v15.94.0) (2026-01-07) ### Bug Fixes * add company filters to project ([d6511b0](d6511b0)) * **journal entry:** use submission_queue to perform submit and cancel actions for rows over 100 ([1d58e9b](1d58e9b)) * not able to submit backdated stock reco ([4b60979](4b60979)) * precision issue causing reservation error ([2d49cc9](2d49cc9)) * resolve conflict ([dbd2964](dbd2964)) * SABB not cancelled on cancel of Stock Reco ([eebd885](eebd885)) * **stock:** prevent excess stock reservation ([4d31012](4d31012)) * **stock:** remove item image to avoid setting the image of previous item ([6f1cfdb](6f1cfdb)) * **trial balance party:** add check for parties with zero credit and debit ([a0566c9](a0566c9)) * update filters on period closing voucher ([728a8b0](728a8b0)) ### Features * add default-age-range in accounts settings (backport [#51458](#51458)) ([#51531](#51531)) ([582db48](582db48)) * allow data import for asset repair doctype ([dc10ef4](dc10ef4))
Description:
Added a Default Age Range field on Accounts Settings to avoid repetitive custom configuration on
fixes: #45830
Age-Range.mp4
Backport needed for v15